On Sat, 11 Jan 2020 18:40:56 +0100, mickflemm@xxxxxxxxx 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@xxxxxxxxx> 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@xxxxxxxxx> > + * > + * 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@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
