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