alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c
@ 2020-01-11 17:40 mickflemm
  2020-01-12  8:42 ` Takashi Iwai
  2020-01-12 22:15 ` kbuild test robot
  0 siblings, 2 replies; 3+ messages in thread
From: mickflemm @ 2020-01-11 17:40 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, Nick Kossifidis

This patch adds support for Presonus Studio 1810c, a usb interface
that's UAC2 compliant with a few quirks and a few extra hw-specific
controls. I've tested all 3 altsettings and the added switch
controls and they work as expected.

More infos on the card:
https://www.presonus.com/products/Studio-1810c

Note that this work is based on packet inspection with
usbmon. I just wanted to get this card to work for using
it on our open-source radio station:
https://github.com/UoC-Radio

v2 address issues reported by Takashi:
* Properly get/set enum type controls
* Prevent race condition on switch_get/set
* Various control naming changes
* Various coding style fixes

Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
---
 sound/usb/Makefile       |   1 +
 sound/usb/format.c       |  17 ++
 sound/usb/mixer_quirks.c |   5 +
 sound/usb/mixer_s1810c.c | 595 +++++++++++++++++++++++++++++++++++++++
 sound/usb/mixer_s1810c.h |   7 +
 sound/usb/quirks.c       |  36 +++
 6 files changed, 661 insertions(+)
 create mode 100644 sound/usb/mixer_s1810c.c
 create mode 100644 sound/usb/mixer_s1810c.h

diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 78edd7d2f..56031026b 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -13,6 +13,7 @@ snd-usb-audio-objs := 	card.o \
 			mixer_scarlett.o \
 			mixer_scarlett_gen2.o \
 			mixer_us16x08.o \
+			mixer_s1810c.o \
 			pcm.o \
 			power.o \
 			proc.o \
diff --git a/sound/usb/format.c b/sound/usb/format.c
index d79db7130..edf3f2a55 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -262,6 +262,23 @@ static int parse_uac2_sample_rate_range(struct snd_usb_audio *chip,
 		}
 
 		for (rate = min; rate <= max; rate += res) {
+
+			/*
+			 * Presonus Studio 1810c anounces invalid
+			 * sampling rates for its streams.
+			 */
+			if (chip->usb_id == USB_ID(0x0194f, 0x010c) &&
+			((rate > 48000 && fp->altsetting == 1) ||
+			 ((rate < 88200 || rate > 96000)
+			  && fp->altsetting == 2) ||
+			 ((rate < 176400 || rate > 192000)
+			  && fp->altsetting == 3))) {
+				if (res)
+					continue;
+				else
+					break;
+			}
+
 			if (fp->rate_table)
 				fp->rate_table[nr_rates] = rate;
 			if (!fp->rate_min || rate < fp->rate_min)
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index 39e27ae6c..71e705fab 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -34,6 +34,7 @@
 #include "mixer_scarlett.h"
 #include "mixer_scarlett_gen2.h"
 #include "mixer_us16x08.h"
+#include "mixer_s1810c.h"
 #include "helper.h"
 
 struct std_mono_table {
@@ -2277,6 +2278,10 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
 	case USB_ID(0x2a39, 0x3fd4): /* RME */
 		err = snd_rme_controls_create(mixer);
 		break;
+
+	case USB_ID(0x0194f, 0x010c): /* Presonus Studio 1810c */
+		err = snd_sc1810_init_mixer(mixer);
+		break;
 	}
 
 	return err;
diff --git a/sound/usb/mixer_s1810c.c b/sound/usb/mixer_s1810c.c
new file mode 100644
index 000000000..ae3929e1c
--- /dev/null
+++ b/sound/usb/mixer_s1810c.c
@@ -0,0 +1,595 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Presonus Studio 1810c driver for ALSA
+ * Copyright (C) 2019 Nick Kossifidis <mickflemm@gmail.com>
+ *
+ * Based on reverse engineering of the communication protocol
+ * between the windows driver / Univeral Control (UC) program
+ * and the device, through usbmon.
+ *
+ * For now this bypasses the mixer, with all channels split,
+ * so that the software can mix with greater flexibility.
+ * It also adds controls for the 4 buttons on the front of
+ * the device.
+ */
+
+#include <linux/usb.h>
+#include <linux/usb/audio-v2.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/control.h>
+
+#include "usbaudio.h"
+#include "mixer.h"
+#include "mixer_quirks.h"
+#include "helper.h"
+
+#define SC1810C_CMD_REQ	160
+#define SC1810C_CMD_REQTYPE \
+	(USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT)
+#define	SC1810C_CMD_F1	0x50617269
+#define	SC1810C_CMD_F2	0x14
+
+/*
+ * DISCLAIMER: These are just guesses based on the
+ * dumps I got.
+ *
+ * It seems like a selects between
+ * device (0), mixer (0x64) and output (0x65)
+ *
+ * For mixer (0x64):
+ *  * b selects an input channel (see below).
+ *  * c selects an output channel pair (see below).
+ *  * d selects left (0) or right (1) of that pair.
+ *  * e 0-> disconnect, 0x01000000-> connect,
+ *	0x0109-> used for stereo-linking channels,
+ *	e is also used for setting volume levels
+ *	in which case b is also set so I guess
+ *	this way it is possible to set the volume
+ *	level from the specified input to the
+ *	specified output.
+ *
+ * IN Channels:
+ * 0 - 7   Mic/Inst/Line (Analog inputs)
+ * 8 - 9   S/PDIF
+ * 10 - 17 ADAT
+ * 18 - 35 DAW (Inputs from the host)
+ *
+ * OUT Channels (pairs):
+ * 0 -> Main out
+ * 1 -> Line1/2
+ * 2 -> Line3/4
+ * 3 -> S/PDIF
+ * 4 -> ADAT?
+ *
+ * For device (0):
+ *  * b and c are not used, at least not on the
+ *    dumps I got.
+ *  * d sets the control id to be modified
+ *    (see below).
+ *  * e sets the setting for that control.
+ *    (so for the switches I was interested
+ *    in it's 0/1)
+ *
+ * For output (0x65):
+ *   * b is the output channel (see above).
+ *   * c is zero.
+ *   * e I guess the same as with mixer except 0x0109
+ *	 which I didn't see in my dumps.
+ *
+ * The two fixed fields have the same values for
+ * mixer and output but a different set for device.
+ */
+struct s1810c_ctl_packet {
+	u32 a;
+	u32 b;
+	u32 fixed1;
+	u32 fixed2;
+	u32 c;
+	u32 d;
+	u32 e;
+};
+
+#define	SC1810C_CTL_LINE_SW	0
+#define	SC1810C_CTL_MUTE_SW	1
+#define SC1810C_CTL_AB_SW	3
+#define	SC1810C_CTL_48V_SW	4
+
+#define SC1810C_SET_STATE_REQ 161
+#define	SC1810C_SET_STATE_REQTYPE SC1810C_CMD_REQTYPE
+#define	SC1810C_SET_STATE_F1 0x64656D73
+#define	SC1810C_SET_STATE_F2 0xF4
+
+#define SC1810C_GET_STATE_REQ 162
+#define	SC1810C_GET_STATE_REQTYPE \
+	(USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN)
+#define	SC1810C_GET_STATE_F1 SC1810C_SET_STATE_F1
+#define	SC1810C_GET_STATE_F2 SC1810C_SET_STATE_F2
+
+#define SC1810C_STATE_F1_IDX	2
+#define	SC1810C_STATE_F2_IDX	3
+
+/*
+ * This packet includes mixer volumes and
+ * various other fields, it's an extended
+ * version of ctl_packet, with a and b
+ * being zero and different f1/f2.
+ */
+struct s1810c_state_packet {
+	u32 fields[63];
+};
+
+#define	SC1810C_STATE_48V_SW	58
+#define	SC1810C_STATE_LINE_SW	59
+#define	SC1810C_STATE_MUTE_SW	60
+#define SC1810C_STATE_AB_SW	62
+
+struct s1810_mixer_state {
+	uint16_t seqnum;
+	struct mutex usb_mutex;
+	struct mutex data_mutex;
+};
+
+static int
+snd_s1810c_send_ctl_packet(struct usb_device *dev, u32 a,
+			   u32 b, u32 c, u32 d, u32 e)
+{
+	struct s1810c_ctl_packet pkt = { 0 };
+	int ret = 0;
+
+	pkt.fixed1 = SC1810C_CMD_F1;
+	pkt.fixed2 = SC1810C_CMD_F2;
+
+	pkt.a = a;
+	pkt.b = b;
+	pkt.c = c;
+	pkt.d = d;
+	/*
+	 * Value for settings 0/1 for this
+	 * output channel is always 0 (probably because
+	 * there is no ADAT output on 1810c)
+	 */
+	pkt.e = (c == 4) ? 0 : e;
+
+	ret = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0),
+			      SC1810C_CMD_REQ,
+			      SC1810C_CMD_REQTYPE, 0, 0, &pkt, sizeof(pkt));
+	if (ret < 0) {
+		dev_warn(&dev->dev, "could not send ctl packet\n");
+		return ret;
+	}
+	return 0;
+}
+
+/*
+ * When opening Universal Control the program periodicaly
+ * sends and receives state packets for syncinc state between
+ * the device and the host.
+ *
+ * Note that if we send only the request to get data back we'll
+ * get an error, we need to first send an empty state packet and
+ * then ask to receive a filled. Their seqnumbers must also match.
+ */
+static int
+snd_sc1810c_get_status_field(struct usb_device *dev,
+			     u32 *field, int field_idx, uint16_t *seqnum)
+{
+	struct s1810c_state_packet pkt_out = { 0 };
+	struct s1810c_state_packet pkt_in = { 0 };
+	int ret = 0;
+
+	pkt_out.fields[SC1810C_STATE_F1_IDX] = SC1810C_SET_STATE_F1;
+	pkt_out.fields[SC1810C_STATE_F2_IDX] = SC1810C_SET_STATE_F2;
+	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
+			      SC1810C_SET_STATE_REQ,
+			      SC1810C_SET_STATE_REQTYPE,
+			      (*seqnum), 0, &pkt_out, sizeof(pkt_out));
+	if (ret < 0) {
+		dev_warn(&dev->dev, "could not send state packet (%d)\n", ret);
+		return ret;
+	}
+
+	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
+			      SC1810C_GET_STATE_REQ,
+			      SC1810C_GET_STATE_REQTYPE,
+			      (*seqnum), 0, &pkt_in, sizeof(pkt_in));
+	if (ret < 0) {
+		dev_warn(&dev->dev, "could not get state field %u (%d)\n",
+			 field_idx, ret);
+		return ret;
+	}
+
+	(*field) = pkt_in.fields[field_idx];
+	(*seqnum)++;
+	return 0;
+}
+
+/*
+ * This is what I got when bypassing the mixer with
+ * all channels split. I'm not 100% sure of what's going
+ * on, I could probably clean this up based on my observations
+ * but I prefer to keep the same behavior as the windows driver.
+ */
+static int snd_s1810c_init_mixer_maps(struct snd_usb_audio *chip)
+{
+	u32 a, b, c, e, n, off;
+	struct usb_device *dev = chip->dev;
+
+	/* Set initial volume levels ? */
+	a = 0x64;
+	e = 0xbc;
+	for (n = 0; n < 2; n++) {
+		off = n * 18;
+		for (b = off, c = 0; b < 18 + off; b++) {
+			/* This channel to all outputs ? */
+			for (c = 0; c <= 8; c++) {
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 0, e);
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 1, e);
+			}
+			/* This channel to main output (again) */
+			snd_s1810c_send_ctl_packet(dev, a, b, 0, 0, e);
+			snd_s1810c_send_ctl_packet(dev, a, b, 0, 1, e);
+		}
+		/*
+		 * I noticed on UC that DAW channels have different
+		 * initial volumes (they can go both above and below
+		 * 0db), so this makes sense.
+		 */
+		e = 0xb53bf0;
+	}
+
+	/* Connect analog outputs ? */
+	a = 0x65;
+	e = 0x01000000;
+	for (b = 1; b < 3; b++) {
+		snd_s1810c_send_ctl_packet(dev, a, b, 0, 0, e);
+		snd_s1810c_send_ctl_packet(dev, a, b, 0, 1, e);
+	}
+	snd_s1810c_send_ctl_packet(dev, a, 0, 0, 0, e);
+	snd_s1810c_send_ctl_packet(dev, a, 0, 0, 1, e);
+
+	/* Set initial volume levels for S/PDIF mappings ? */
+	a = 0x64;
+	e = 0xbc;
+	c = 3;
+	for (n = 0; n < 2; n++) {
+		off = n * 18;
+		for (b = off; b < 18 + off; b++) {
+			snd_s1810c_send_ctl_packet(dev, a, b, c, 0, e);
+			snd_s1810c_send_ctl_packet(dev, a, b, c, 1, e);
+		}
+		e = 0xb53bf0;
+	}
+
+	/* Connect S/PDIF output ? */
+	a = 0x65;
+	e = 0x01000000;
+	snd_s1810c_send_ctl_packet(dev, a, 3, 0, 0, e);
+	snd_s1810c_send_ctl_packet(dev, a, 3, 0, 1, e);
+
+	/* Connect all outputs (again) ? */
+	a = 0x65;
+	e = 0x01000000;
+	for (b = 0; b < 4; b++) {
+		snd_s1810c_send_ctl_packet(dev, a, b, 0, 0, e);
+		snd_s1810c_send_ctl_packet(dev, a, b, 0, 1, e);
+	}
+
+	/* Basic routing to get sound out of the device */
+	a = 0x64;
+	e = 0x01000000;
+	for (c = 0; c < 4; c++) {
+		for (b = 0; b < 36; b++) {
+			if ((c == 0 && b == 18) ||	/* DAW1/2 -> Main */
+			    (c == 1 && b == 20) ||	/* DAW3/4 -> Line3/4 */
+			    (c == 2 && b == 22) ||	/* DAW4/5 -> Line5/4 */
+			    (c == 3 && b == 24)) {	/* DAW5/6 -> S/PDIF */
+				/* Left */
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 0, e);
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 1, 0);
+				b++;
+				/* Right */
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 0, 0);
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 1, e);
+			} else {
+				/* Leave the rest disconnected */
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 0, 0);
+				snd_s1810c_send_ctl_packet(dev, a, b, c, 1, 0);
+			}
+		}
+	}
+
+	/* Set initial volume levels for S/PDIF (again) ? */
+	a = 0x64;
+	e = 0xbc;
+	c = 3;
+	for (n = 0; n < 2; n++) {
+		off = n * 18;
+		for (b = off; b < 18 + off; b++) {
+			snd_s1810c_send_ctl_packet(dev, a, b, c, 0, e);
+			snd_s1810c_send_ctl_packet(dev, a, b, c, 1, e);
+		}
+		e = 0xb53bf0;
+	}
+
+	/* Connect S/PDIF outputs (again) ? */
+	a = 0x65;
+	e = 0x01000000;
+	snd_s1810c_send_ctl_packet(dev, a, 3, 0, 0, e);
+	snd_s1810c_send_ctl_packet(dev, a, 3, 0, 1, e);
+
+	/* Again ? */
+	snd_s1810c_send_ctl_packet(dev, a, 3, 0, 0, e);
+	snd_s1810c_send_ctl_packet(dev, a, 3, 0, 1, e);
+
+	return 0;
+}
+
+/*
+ * Sync state with the device and retrieve the requested field,
+ * whose index is specified in (kctl->private_value & 0xFF),
+ * from the received fields array.
+ */
+static int
+snd_s1810c_get_switch_state(struct usb_mixer_interface *mixer,
+			    struct snd_kcontrol *kctl, u32 *state)
+{
+	struct snd_usb_audio *chip = mixer->chip;
+	struct s1810_mixer_state *private = mixer->private_data;
+	u32 field = 0;
+	u32 ctl_idx = (u32) (kctl->private_value & 0xFF);
+	int ret = 0;
+
+	mutex_lock(&private->usb_mutex);
+	ret = snd_sc1810c_get_status_field(chip->dev, &field,
+					   ctl_idx, &private->seqnum);
+	if (ret < 0)
+		goto unlock;
+
+	*state = field;
+ unlock:
+	mutex_unlock(&private->usb_mutex);
+	return ret ? ret : 0;
+}
+
+/*
+ * Send a control packet to the device for the control id
+ * specified in (kctl->private_value >> 8) with value
+ * specified in (kctl->private_value >> 16).
+ */
+static int
+snd_s1810c_set_switch_state(struct usb_mixer_interface *mixer,
+			    struct snd_kcontrol *kctl)
+{
+	struct snd_usb_audio *chip = mixer->chip;
+	struct s1810_mixer_state *private = mixer->private_data;
+	u32 pval = (u32) kctl->private_value;
+	u32 ctl_id = (pval >> 8) & 0xFF;
+	u32 ctl_val = (pval >> 16) & 0x1;
+	int ret = 0;
+
+	mutex_lock(&private->usb_mutex);
+	ret = snd_s1810c_send_ctl_packet(chip->dev, 0, 0, 0, ctl_id, ctl_val);
+	mutex_unlock(&private->usb_mutex);
+	return ret;
+}
+
+/* Generic get/set/init functions for switch controls */
+
+static int
+snd_s1810c_switch_get(struct snd_kcontrol *kctl,
+		      struct snd_ctl_elem_value *ctl_elem)
+{
+	struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
+	struct usb_mixer_interface *mixer = list->mixer;
+	struct s1810_mixer_state *private = mixer->private_data;
+	u32 pval = (u32) kctl->private_value;
+	u32 ctl_idx = pval & 0xFF;
+	u32 state = 0;
+	int ret = 0;
+
+	mutex_lock(&private->data_mutex);
+	ret = snd_s1810c_get_switch_state(mixer, kctl, &state);
+	if (ret < 0)
+		goto unlock;
+
+	switch (ctl_idx) {
+	case SC1810C_STATE_LINE_SW:
+	case SC1810C_STATE_AB_SW:
+		ctl_elem->value.enumerated.item[0] = (int)state;
+		break;
+	default:
+		ctl_elem->value.integer.value[0] = (long)state;
+	}
+
+ unlock:
+	mutex_unlock(&private->data_mutex);
+	return (ret < 0) ? ret : 0;
+}
+
+static int
+snd_s1810c_switch_set(struct snd_kcontrol *kctl,
+		      struct snd_ctl_elem_value *ctl_elem)
+{
+	struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
+	struct usb_mixer_interface *mixer = list->mixer;
+	struct s1810_mixer_state *private = mixer->private_data;
+	u32 pval = (u32) kctl->private_value;
+	u32 ctl_idx = pval & 0xFF;
+	u32 curval = 0;
+	u32 newval = 0;
+	int ret = 0;
+
+	mutex_lock(&private->data_mutex);
+	ret = snd_s1810c_get_switch_state(mixer, kctl, &curval);
+	if (ret < 0)
+		goto unlock;
+
+	switch (ctl_idx) {
+	case SC1810C_STATE_LINE_SW:
+	case SC1810C_STATE_AB_SW:
+		newval = (u32) ctl_elem->value.enumerated.item[0];
+		break;
+	default:
+		newval = (u32) ctl_elem->value.integer.value[0];
+	}
+
+	if (curval == newval)
+		goto unlock;
+
+	kctl->private_value &= ~(0x1 << 16);
+	kctl->private_value |= (unsigned int)(newval & 0x1) << 16;
+	ret = snd_s1810c_set_switch_state(mixer, kctl);
+
+ unlock:
+	mutex_unlock(&private->data_mutex);
+	return (ret < 0) ? 0 : 1;
+}
+
+static int
+snd_s1810c_switch_init(struct usb_mixer_interface *mixer,
+		       const struct snd_kcontrol_new *new_kctl)
+{
+	struct snd_kcontrol *kctl;
+	struct usb_mixer_elem_info *elem;
+
+	elem = kzalloc(sizeof(struct usb_mixer_elem_info), GFP_KERNEL);
+	if (!elem)
+		return -ENOMEM;
+
+	elem->head.mixer = mixer;
+	elem->control = 0;
+	elem->head.id = 0;
+	elem->channels = 1;
+
+	kctl = snd_ctl_new1(new_kctl, elem);
+	if (!kctl) {
+		kfree(elem);
+		return -ENOMEM;
+	}
+	kctl->private_free = snd_usb_mixer_elem_free;
+
+	return snd_usb_mixer_add_control(&elem->head, kctl);
+}
+
+static int
+snd_s1810c_line_sw_info(struct snd_kcontrol *kctl,
+			struct snd_ctl_elem_info *uinfo)
+{
+	static const char *const texts[2] = {
+		"Preamp On (Mic/Inst)",
+		"Preamp Off (Line in)"
+	};
+
+	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
+}
+
+static const struct snd_kcontrol_new snd_s1810c_line_sw = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "Line 1/2 Source Type Switch",
+	.info = snd_s1810c_line_sw_info,
+	.get = snd_s1810c_switch_get,
+	.put = snd_s1810c_switch_set,
+	.private_value = (SC1810C_STATE_LINE_SW | SC1810C_CTL_LINE_SW << 8)
+};
+
+static const struct snd_kcontrol_new snd_s1810c_mute_sw = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "Mute Main Out Switch",
+	.info = snd_ctl_boolean_mono_info,
+	.get = snd_s1810c_switch_get,
+	.put = snd_s1810c_switch_set,
+	.private_value = (SC1810C_STATE_MUTE_SW | SC1810C_CTL_MUTE_SW << 8)
+};
+
+static const struct snd_kcontrol_new snd_s1810c_48v_sw = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "48V Phantom Power On Mic Inputs Switch",
+	.info = snd_ctl_boolean_mono_info,
+	.get = snd_s1810c_switch_get,
+	.put = snd_s1810c_switch_set,
+	.private_value = (SC1810C_STATE_48V_SW | SC1810C_CTL_48V_SW << 8)
+};
+
+static int
+snd_s1810c_ab_sw_info(struct snd_kcontrol *kctl,
+		      struct snd_ctl_elem_info *uinfo)
+{
+	static const char *const texts[2] = {
+		"1/2",
+		"3/4"
+	};
+
+	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
+}
+
+static const struct snd_kcontrol_new snd_s1810c_ab_sw = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "Headphone 1 Source Route",
+	.info = snd_s1810c_ab_sw_info,
+	.get = snd_s1810c_switch_get,
+	.put = snd_s1810c_switch_set,
+	.private_value = (SC1810C_STATE_AB_SW | SC1810C_CTL_AB_SW << 8)
+};
+
+static void snd_sc1810_mixer_state_free(struct usb_mixer_interface *mixer)
+{
+	struct s1810_mixer_state *private = mixer->private_data;
+	kfree(private);
+	mixer->private_data = NULL;
+}
+
+/* Entry point, called from mixer_quirks.c */
+int snd_sc1810_init_mixer(struct usb_mixer_interface *mixer)
+{
+	struct s1810_mixer_state *private = NULL;
+	struct snd_usb_audio *chip = mixer->chip;
+	struct usb_device *dev = chip->dev;
+	int ret = 0;
+
+	/* Run this only once */
+	if (!list_empty(&chip->mixer_list))
+		return 0;
+
+	dev_info(&dev->dev,
+		 "Presonus Studio 1810c, device_setup: %u\n", chip->setup);
+	if (chip->setup == 1)
+		dev_info(&dev->dev, "(8out/18in @ 48KHz)\n");
+	else if (chip->setup == 2)
+		dev_info(&dev->dev, "(6out/8in @ 192KHz)\n");
+	else
+		dev_info(&dev->dev, "(8out/14in @ 96KHz)\n");
+
+	ret = snd_s1810c_init_mixer_maps(chip);
+	if (ret < 0)
+		return ret;
+
+	private = kzalloc(sizeof(struct s1810_mixer_state), GFP_KERNEL);
+	if (!private)
+		return -ENOMEM;
+
+	mutex_init(&private->usb_mutex);
+	mutex_init(&private->data_mutex);
+
+	mixer->private_data = private;
+	mixer->private_free = snd_sc1810_mixer_state_free;
+
+	private->seqnum = 1;
+
+	ret = snd_s1810c_switch_init(mixer, &snd_s1810c_line_sw);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_s1810c_switch_init(mixer, &snd_s1810c_mute_sw);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_s1810c_switch_init(mixer, &snd_s1810c_48v_sw);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_s1810c_switch_init(mixer, &snd_s1810c_ab_sw);
+	if (ret < 0)
+		return ret;
+	return ret;
+}
diff --git a/sound/usb/mixer_s1810c.h b/sound/usb/mixer_s1810c.h
new file mode 100644
index 000000000..a79a3743c
--- /dev/null
+++ b/sound/usb/mixer_s1810c.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Presonus Studio 1810c driver for ALSA
+ * Copyright (C) 2019 Nick Kossifidis <mickflemm@gmail.com>
+ */
+
+int snd_sc1810_init_mixer(struct usb_mixer_interface *mixer);
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index a81c20664..9bbdea882 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -1227,6 +1227,38 @@ static int fasttrackpro_skip_setting_quirk(struct snd_usb_audio *chip,
 	return 0; /* keep this altsetting */
 }
 
+static int s1810c_skip_setting_quirk(struct snd_usb_audio *chip,
+					   int iface, int altno)
+{
+	/*
+	 * Altno settings:
+	 *
+	 * Playback (Interface 1):
+	 * 1: 6 Analog + 2 S/PDIF
+	 * 2: 6 Analog + 2 S/PDIF
+	 * 3: 6 Analog
+	 *
+	 * Capture (Interface 2):
+	 * 1: 8 Analog + 2 S/PDIF + 8 ADAT
+	 * 2: 8 Analog + 2 S/PDIF + 4 ADAT
+	 * 3: 8 Analog
+	 */
+
+	/*
+	 * I'll leave 2 as the default one and
+	 * use device_setup to switch to the
+	 * other two.
+	 */
+	if ((chip->setup == 0 || chip->setup > 2) && altno != 2)
+		return 1;
+	else if (chip->setup == 1 && altno != 1)
+		return 1;
+	else if (chip->setup == 2 && altno != 3)
+		return 1;
+
+	return 0;
+}
+
 int snd_usb_apply_interface_quirk(struct snd_usb_audio *chip,
 				  int iface,
 				  int altno)
@@ -1240,6 +1272,10 @@ int snd_usb_apply_interface_quirk(struct snd_usb_audio *chip,
 	/* fasttrackpro usb: skip altsets incompatible with device_setup */
 	if (chip->usb_id == USB_ID(0x0763, 0x2012))
 		return fasttrackpro_skip_setting_quirk(chip, iface, altno);
+	/* presonus studio 1810c: skip altsets incompatible with device_setup */
+	if (chip->usb_id == USB_ID(0x0194f, 0x010c))
+		return s1810c_skip_setting_quirk(chip, iface, altno);
+
 
 	return 0;
 }
-- 
2.23.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c
  2020-01-11 17:40 [alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c mickflemm
@ 2020-01-12  8:42 ` Takashi Iwai
  2020-01-12 22:15 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2020-01-12  8:42 UTC (permalink / raw)
  To: mickflemm; +Cc: alsa-devel

On Sat, 11 Jan 2020 18:40:56 +0100,
mickflemm@gmail.com wrote:
> 
> This patch adds support for Presonus Studio 1810c, a usb interface
> that's UAC2 compliant with a few quirks and a few extra hw-specific
> controls. I've tested all 3 altsettings and the added switch
> controls and they work as expected.
> 
> More infos on the card:
> https://www.presonus.com/products/Studio-1810c
> 
> Note that this work is based on packet inspection with
> usbmon. I just wanted to get this card to work for using
> it on our open-source radio station:
> https://github.com/UoC-Radio
> 
> v2 address issues reported by Takashi:
> * Properly get/set enum type controls
> * Prevent race condition on switch_get/set
> * Various control naming changes
> * Various coding style fixes
> 
> Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>

Thanks, the patch looks almost good for merge, just a couple of
nitpicks:

> --- a/sound/usb/format.c
> +++ b/sound/usb/format.c
> @@ -262,6 +262,23 @@ static int parse_uac2_sample_rate_range(struct snd_usb_audio *chip,
>  		}
>  
>  		for (rate = min; rate <= max; rate += res) {
> +
> +			/*
> +			 * Presonus Studio 1810c anounces invalid
> +			 * sampling rates for its streams.
> +			 */
> +			if (chip->usb_id == USB_ID(0x0194f, 0x010c) &&
> +			((rate > 48000 && fp->altsetting == 1) ||
> +			 ((rate < 88200 || rate > 96000)
> +			  && fp->altsetting == 2) ||
> +			 ((rate < 176400 || rate > 192000)
> +			  && fp->altsetting == 3))) {

I still find it too hard to read.  Maybe better to factor out this
check into a function.  Also it's clearer in a form of something like:

static bool sc1810c_valid_sample_rate()
{
	switch (fp->altsetting) {
	case 1:
		return rate <= 48000;
	case 2:
		return rate >= 88200 && rate <= 96000;
	case 3:
		return rate >= 176400 && rate <= 192000;
	default:
		return true;
	}
}

> --- /dev/null
> +++ b/sound/usb/mixer_s1810c.c
> @@ -0,0 +1,595 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Presonus Studio 1810c driver for ALSA
> + * Copyright (C) 2019 Nick Kossifidis <mickflemm@gmail.com>
> + *
> + * Based on reverse engineering of the communication protocol
> + * between the windows driver / Univeral Control (UC) program
> + * and the device, through usbmon.
> + *
> + * For now this bypasses the mixer, with all channels split,
> + * so that the software can mix with greater flexibility.
> + * It also adds controls for the 4 buttons on the front of
> + * the device.
> + */
> +
> +#include <linux/usb.h>
> +#include <linux/usb/audio-v2.h>
> +#include <linux/slab.h>
> +#include <sound/core.h>
> +#include <sound/control.h>
> +
> +#include "usbaudio.h"
> +#include "mixer.h"
> +#include "mixer_quirks.h"
> +#include "helper.h"

Include mixer_s1810c.h here, too, for the function declaration.

> +static const struct snd_kcontrol_new snd_s1810c_line_sw = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +	.name = "Line 1/2 Source Type Switch",

This one is with an enum, so better to avoid "Switch" suffix.
The user-space expects a "Switch" suffix is for a boolean blindly.
Either omit the switch suffix, or use "Route" suffix, in any.

Also, when I apply with git-am, the author is taken from From: line,
which doesn't contain your full name .  The best would to submit the
patch with git-send-email, then it'll add a proper From: line if
needed.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c
  2020-01-11 17:40 [alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c mickflemm
  2020-01-12  8:42 ` Takashi Iwai
@ 2020-01-12 22:15 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2020-01-12 22:15 UTC (permalink / raw)
  To: mickflemm; +Cc: tiwai, alsa-devel, kbuild-all, Nick Kossifidis

[-- Attachment #1: Type: text/plain, Size: 3384 bytes --]

Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on sound/for-next]
[also build test WARNING on v5.5-rc5 next-20200110]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/mickflemm-gmail-com/ALSA-usb-audio-Add-support-for-Presonus-Studio-1810c/20200112-014426
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: x86_64-randconfig-s2-20200113 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.2-10+deb8u1) 4.9.2
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   sound/usb/mixer_s1810c.c: In function 'snd_sc1810c_get_status_field':
>> sound/usb/mixer_s1810c.c:177:9: warning: missing braces around initializer [-Wmissing-braces]
     struct s1810c_state_packet pkt_out = { 0 };
            ^
   sound/usb/mixer_s1810c.c:177:9: warning: (near initialization for 'pkt_out.fields') [-Wmissing-braces]
   sound/usb/mixer_s1810c.c:178:9: warning: missing braces around initializer [-Wmissing-braces]
     struct s1810c_state_packet pkt_in = { 0 };
            ^
   sound/usb/mixer_s1810c.c:178:9: warning: (near initialization for 'pkt_in.fields') [-Wmissing-braces]

vim +177 sound/usb/mixer_s1810c.c

   163	
   164	/*
   165	 * When opening Universal Control the program periodicaly
   166	 * sends and receives state packets for syncinc state between
   167	 * the device and the host.
   168	 *
   169	 * Note that if we send only the request to get data back we'll
   170	 * get an error, we need to first send an empty state packet and
   171	 * then ask to receive a filled. Their seqnumbers must also match.
   172	 */
   173	static int
   174	snd_sc1810c_get_status_field(struct usb_device *dev,
   175				     u32 *field, int field_idx, uint16_t *seqnum)
   176	{
 > 177		struct s1810c_state_packet pkt_out = { 0 };
   178		struct s1810c_state_packet pkt_in = { 0 };
   179		int ret = 0;
   180	
   181		pkt_out.fields[SC1810C_STATE_F1_IDX] = SC1810C_SET_STATE_F1;
   182		pkt_out.fields[SC1810C_STATE_F2_IDX] = SC1810C_SET_STATE_F2;
   183		ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
   184				      SC1810C_SET_STATE_REQ,
   185				      SC1810C_SET_STATE_REQTYPE,
   186				      (*seqnum), 0, &pkt_out, sizeof(pkt_out));
   187		if (ret < 0) {
   188			dev_warn(&dev->dev, "could not send state packet (%d)\n", ret);
   189			return ret;
   190		}
   191	
   192		ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
   193				      SC1810C_GET_STATE_REQ,
   194				      SC1810C_GET_STATE_REQTYPE,
   195				      (*seqnum), 0, &pkt_in, sizeof(pkt_in));
   196		if (ret < 0) {
   197			dev_warn(&dev->dev, "could not get state field %u (%d)\n",
   198				 field_idx, ret);
   199			return ret;
   200		}
   201	
   202		(*field) = pkt_in.fields[field_idx];
   203		(*seqnum)++;
   204		return 0;
   205	}
   206	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35591 bytes --]

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-01-12 22:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-11 17:40 [alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c mickflemm
2020-01-12  8:42 ` Takashi Iwai
2020-01-12 22:15 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).