January 2020 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH] ALSA: cmipci: Fix possible a data race in snd_cmipci_interrupt()
by Jia-Ju Bai 13 Jan '20

13 Jan '20

The functions snd_cmipci_interrupt() and snd_cmipci_capture_trigger() may be concurrently executed. The function snd_cmipci_capture_trigger() calls snd_cmipci_pcm_trigger(). In snd_cmipci_pcm_trigger(), the variable rec->running is written with holding a spinlock cm->reg_lock. But in snd_cmipci_interrupt(), the identical variable cm->channel[0].running or cm->channel[1].running is read without holding this spinlock. Thus, a possible data race may occur. To fix this data race, in snd_cmipci_interrupt(), the variables cm->channel[0].running and cm->channel[1].running are read with holding the spinlock cm->reg_lock. This data race is found by the runtime testing of our tool DILP-2. Signed-off-by: Jia-Ju Bai <baijiaju1990(a)gmail.com> --- sound/pci/cmipci.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c index dd9d62e2b633..f791152ec48f 100644 --- a/sound/pci/cmipci.c +++ b/sound/pci/cmipci.c @@ -1430,7 +1430,7 @@ static int snd_cmipci_capture_spdif_hw_free(struct snd_pcm_substream *subs) static irqreturn_t snd_cmipci_interrupt(int irq, void *dev_id) { struct cmipci *cm = dev_id; - unsigned int status, mask = 0; + unsigned int run_flag0, run_flag1, status, mask = 0; /* fastpath out, to ease interrupt sharing */ status = snd_cmipci_read(cm, CM_REG_INT_STATUS); @@ -1445,15 +1445,17 @@ static irqreturn_t snd_cmipci_interrupt(int irq, void *dev_id) mask |= CM_CH1_INT_EN; snd_cmipci_clear_bit(cm, CM_REG_INT_HLDCLR, mask); snd_cmipci_set_bit(cm, CM_REG_INT_HLDCLR, mask); + run_flag0 = cm->channel[0].running; + run_flag1 = cm->channel[1].running; spin_unlock(&cm->reg_lock); if (cm->rmidi && (status & CM_UARTINT)) snd_mpu401_uart_interrupt(irq, cm->rmidi->private_data); if (cm->pcm) { - if ((status & CM_CHINT0) && cm->channel[0].running) + if ((status & CM_CHINT0) && run_flag0) snd_pcm_period_elapsed(cm->channel[0].substream); - if ((status & CM_CHINT1) && cm->channel[1].running) + if ((status & CM_CHINT1) && run_flag1) snd_pcm_period_elapsed(cm->channel[1].substream); } return IRQ_HANDLED; -- 2.17.1

2 3

[alsa-devel] [PATCH 0/6] soundwire: stream: fix state machines and transitions
by Pierre-Louis Bossart 13 Jan '20

13 Jan '20

The existing stream support works fine with simple cases, but does not map well with ALSA transitions for underflows/resume where prepare() can be called multiple times. Concurrency with multiple devices per links or multiple streams enabled on the same link also needs to be fixed. These patches are the result of hours of validation on the Intel side and should benefit other implementations since there is nothing hardware-specific. The Intel-specific changes being reviewed do depend on those stream changes though to be functional. Bard Liao (1): soundwire: stream: only prepare stream when it is configured. Pierre-Louis Bossart (3): soundwire: stream: remove redundant pr_err traces soundwire: stream: update state machine and add state checks soundwire: stream: do not update parameters during DISABLED-PREPARED transition Rander Wang (2): soundwire: stream: fix support for multiple Slaves on the same link soundwire: stream: don't program ports for a stream that has not been prepared Documentation/driver-api/soundwire/stream.rst | 63 ++++++++---- drivers/soundwire/stream.c | 97 +++++++++++++++---- 2 files changed, 124 insertions(+), 36 deletions(-) base-commit: 09f6a72d014386939d21899921dd379006471a4b -- 2.20.1

2 15

[alsa-devel] [PATCH v2] ALSA: usb-audio: Add support for Presonus Studio 1810c
by mickflemm＠gmail.com 12 Jan '20

12 Jan '20

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(a)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(a)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(a)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

3 2

[alsa-devel] [PATCH v2] ALSA: usb-audio: Add boot quirk for MOTU M Series
by Alexander Tsoy 12 Jan '20

12 Jan '20

Add delay to make sure that audio urbs are not sent too early. Otherwise the device hangs. Windows driver makes ~2s delay, so use about the same time delay value. snd_usb_apply_boot_quirk() is called 3 times for my MOTU M4, which is an overkill. Thus a quirk that is called only once is implemented. Also send two vendor-specific control messages before and after the delay. This behaviour is blindly copied from the Windows driver. Signed-off-by: Alexander Tsoy <alexander(a)tsoy.me> --- v2 changes: indentation fixes sound/usb/card.c | 4 ++++ sound/usb/quirks.c | 38 ++++++++++++++++++++++++++++++++++++++ sound/usb/quirks.h | 5 +++++ 3 files changed, 47 insertions(+) diff --git a/sound/usb/card.c b/sound/usb/card.c index 9f743ebae615..2f582ac7cf78 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -600,6 +600,10 @@ static int usb_audio_probe(struct usb_interface *intf, } } if (! chip) { + err = snd_usb_apply_boot_quirk_once(dev, intf, quirk, id); + if (err < 0) + return err; + /* it's a fresh one. * now look for an empty slot and create a new card instance */ diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 82184036437b..0464df785f2e 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1113,6 +1113,31 @@ static int snd_usb_motu_microbookii_boot_quirk(struct usb_device *dev) return err; } +static int snd_usb_motu_m_series_boot_quirk(struct usb_device *dev) +{ + int ret; + + if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0))) + return -EINVAL; + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), + 1, USB_TYPE_VENDOR | USB_RECIP_DEVICE, + 0x0, 0, NULL, 0, 1000); + + if (ret < 0) + return ret; + + msleep(2000); + + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), + 1, USB_TYPE_VENDOR | USB_RECIP_DEVICE, + 0x20, 0, NULL, 0, 1000); + + if (ret < 0) + return ret; + + return 0; +} + /* * Setup quirks */ @@ -1297,6 +1322,19 @@ int snd_usb_apply_boot_quirk(struct usb_device *dev, return 0; } +int snd_usb_apply_boot_quirk_once(struct usb_device *dev, + struct usb_interface *intf, + const struct snd_usb_audio_quirk *quirk, + unsigned int id) +{ + switch (id) { + case USB_ID(0x07fd, 0x0008): /* MOTU M Series */ + return snd_usb_motu_m_series_boot_quirk(dev); + } + + return 0; +} + /* * check if the device uses big-endian samples */ diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h index a80e0ddd0736..b46b108769e8 100644 --- a/sound/usb/quirks.h +++ b/sound/usb/quirks.h @@ -20,6 +20,11 @@ int snd_usb_apply_boot_quirk(struct usb_device *dev, const struct snd_usb_audio_quirk *quirk, unsigned int usb_id); +int snd_usb_apply_boot_quirk_once(struct usb_device *dev, + struct usb_interface *intf, + const struct snd_usb_audio_quirk *quirk, + unsigned int usb_id); + void snd_usb_set_format_quirk(struct snd_usb_substream *subs, struct audioformat *fmt); -- 2.24.1

1 0

[alsa-devel] [PATCH] ALSA: usb-audio: Add boot quirk for MOTU M Series
by Alexander Tsoy 12 Jan '20

12 Jan '20

Add delay to make sure that audio urbs are not sent too early. Otherwise the device hangs. Windows driver makes ~2s delay, so use about the same time delay value. snd_usb_apply_boot_quirk() is called 3 times for my MOTU M4, which is an overkill. Thus a quirk that is called only once is implemented. Also send two vendor-specific control messages before and after the delay. This behaviour is blindly copied from the Windows driver. Signed-off-by: Alexander Tsoy <alexander(a)tsoy.me> --- sound/usb/card.c | 4 ++++ sound/usb/quirks.c | 38 ++++++++++++++++++++++++++++++++++++++ sound/usb/quirks.h | 5 +++++ 3 files changed, 47 insertions(+) diff --git a/sound/usb/card.c b/sound/usb/card.c index 9f743ebae615..2f582ac7cf78 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -600,6 +600,10 @@ static int usb_audio_probe(struct usb_interface *intf, } } if (! chip) { + err = snd_usb_apply_boot_quirk_once(dev, intf, quirk, id); + if (err < 0) + return err; + /* it's a fresh one. * now look for an empty slot and create a new card instance */ diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 82184036437b..c93c53126243 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1113,6 +1113,31 @@ static int snd_usb_motu_microbookii_boot_quirk(struct usb_device *dev) return err; } +static int snd_usb_motu_m_series_boot_quirk(struct usb_device *dev) +{ + int ret; + + if (snd_usb_pipe_sanity_check(dev, usb_sndctrlpipe(dev, 0))) + return -EINVAL; + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), + 1, USB_TYPE_VENDOR | USB_RECIP_DEVICE, + 0x0, 0, NULL, 0, 1000); + + if (ret < 0) + return ret; + + msleep(2000); + + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), + 1, USB_TYPE_VENDOR | USB_RECIP_DEVICE, + 0x20, 0, NULL, 0, 1000); + + if (ret < 0) + return ret; + + return 0; +} + /* * Setup quirks */ @@ -1297,6 +1322,19 @@ int snd_usb_apply_boot_quirk(struct usb_device *dev, return 0; } +int snd_usb_apply_boot_quirk_once(struct usb_device *dev, + struct usb_interface *intf, + const struct snd_usb_audio_quirk *quirk, + unsigned int id) +{ + switch (id) { + case USB_ID(0x07fd, 0x0008): /* MOTU M Series */ + return snd_usb_motu_m_series_boot_quirk(dev); + } + + return 0; +} + /* * check if the device uses big-endian samples */ diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h index a80e0ddd0736..fa83a98ad7bb 100644 --- a/sound/usb/quirks.h +++ b/sound/usb/quirks.h @@ -20,6 +20,11 @@ int snd_usb_apply_boot_quirk(struct usb_device *dev, const struct snd_usb_audio_quirk *quirk, unsigned int usb_id); +int snd_usb_apply_boot_quirk_once(struct usb_device *dev, + struct usb_interface *intf, + const struct snd_usb_audio_quirk *quirk, + unsigned int usb_id); + void snd_usb_set_format_quirk(struct snd_usb_substream *subs, struct audioformat *fmt); -- 2.24.1

2 1

[alsa-devel] [PATCH 0/2] ALSA: fix compiler warnings
by Pierre-Louis Bossart 12 Jan '20

12 Jan '20

2 additional issues reported by GCC W=1. the first one looks like a bug, the second one more of a style issue. Fixes for ASoC will be provided separately after review/test. Pierre-Louis Bossart (2): ALSA: HDA: patch_realtek: fix empty macro usage in if block ALSA: usb: update old-style static const declaration sound/pci/hda/patch_realtek.c | 4 ++-- sound/usb/mixer_quirks.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.20.1

2 4

[alsa-devel] [PATCH] ALSA: timer: fix nsec/sec initialization confusion
by Pierre-Louis Bossart 12 Jan '20

12 Jan '20

GCC reports a warning with W=1: sound/core/timer.c: In function ‘snd_timer_user_read’: sound/core/timer.c:2219:19: warning: initialized field overwritten [-Woverride-init] 2219 | .tstamp_sec = tread->tstamp_nsec, | ^~~~~ sound/core/timer.c:2219:19: note: (near initialization for ‘(anonymous).tstamp_sec’) Assigning nsec values to sec fields is problematic in general, even more so when the initial goal was to survive the 2030 timer armageddon. Fix by using the proper field in the initialization Cc: Baolin Wang <baolin.wang(a)linaro.org> Cc: Arnd Bergmann <arnd(a)arndb.de> Fixes: 07094ae6f9527 ("ALSA: Avoid using timespec for struct snd_timer_tread") Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> --- sound/core/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/core/timer.c b/sound/core/timer.c index 8835ff91a893..d9f85f2d66a3 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -2216,7 +2216,7 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer, tread32 = (struct snd_timer_tread32) { .event = tread->event, .tstamp_sec = tread->tstamp_sec, - .tstamp_sec = tread->tstamp_nsec, + .tstamp_nsec = tread->tstamp_nsec, .val = tread->val, }; -- 2.20.1

3 2

[alsa-devel] alsa-gobject: ctl: fix wrong operation for values in enumerated type of element
by GitHub pull_request - opened 12 Jan '20

12 Jan '20

alsa-project/alsa-gobject pull request #4 was opened from takaswie: The 'snd_ctl_elem_value' structure includes union member for values. Although it includes 'enumerated' member for values in enumerated type of element, current implementation of alsactl library uses 'integer' member to pick up the values. As a result, in any 64 bit System V ABI, the library fails to get/set values for the enumerated type of element. This commit fixes the bug. Signed-off-by: Takashi Sakamoto <o-takashi(a)sakamocchi.jp> Request URL : https://github.com/alsa-project/alsa-gobject/pull/4 Patch URL : https://github.com/alsa-project/alsa-gobject/pull/4.patch Repository URL: https://github.com/alsa-project/alsa-gobject

1 0

[alsa-devel] When storing settings which contain a [ or ], the setting is not quoted.
by GitHub issues - edited 10 Jan '20

10 Jan '20

alsa-project/alsa-lib issue #22 was edited from Bertware: I am using a Raspberry Pi 1 B+ with a Hifiberry DAC+ADC Pro HAT. This HAT contains a PCM5122 Stereo DAC and an ADC. Now to the issue: The ADC has parameters such as `VINR1[SE]` and `VINR2[SE] + VINR1[SE]`. When using `alsactl store` or`alsactl restore`, restoring will fail due to an invalid file. `ALSA lib conf.c:1887:(_snd_config_load_with_include) _toplevel_:354:30:Unexpected char` This error points to the parameter `VINR1[SE]`, which is not surrounded by quotes. The other parameters, which also contain other special characters such as spaces, are surrounded by quotes. An excerpt of the file is included below. This clearly illustrates which names were encapsulated between quotes. ``` control.28 { iface MIXER name 'ADC Right Capture Source' value VINR1[SE] comment { access 'read write' type ENUMERATED count 1 item.0 'No Select' item.1 VINR1[SE] item.2 VINR2[SE] item.3 'VINR2[SE] + VINR1[SE]' item.4 VINR3[SE] item.5 'VINR3[SE] + VINR1[SE]' item.6 'VINR3[SE] + VINR2[SE]' item.7 'VINR3[SE] + VINR2[SE] + VINR1[SE]' item.8 VINR4[SE] item.9 'VINR4[SE] + VINR1[SE]' item.10 'VINR4[SE] + VINR2[SE]' item.11 'VINR4[SE] + VINR2[SE] + VINR1[SE]' item.12 'VINR4[SE] + VINR3[SE]' item.13 'VINR4[SE] + VINR3[SE] + VINR1[SE]' item.14 'VINR4[SE] + VINR3[SE] + VINR2[SE]' item.15 'VINR4[SE] + VINR3[SE] + VINR2[SE] + VINR1[SE]' item.16 '{VIN2P, VIN2M}[DIFF]' item.17 '{VIN3P, VIN3M}[DIFF]' item.18 '{VIN2P, VIN2M}[DIFF] + {VIN3P, VIN3M}[DIFF]' } } ``` Manually fixing this file by adding '' around the items which miss quotes fixes the issue, and allows restoring it. **How this leads to alsa-lib and where the issue is located** Creation of the output data and writing it to a file is handled by the save_state method in alsactl: https://github.com/alsa-project/alsa-utils/blob/master/alsactl/state.c#L1539 This code in turn calls snd_config_save in the alsa-lib project: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… Then further to string_print: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… And then in this loop which decides which characters need to be quoted: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… **Proposed solution** The characters `[` and `]` should be a reason to put text between quotes. These characters should be added to the switch statement in print_string here: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… Regards, Bert Issue URL : https://github.com/alsa-project/alsa-lib/issues/22 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

[alsa-devel] When storing settings which contain a [ or ], the setting is not quoted.
by GitHub issues - edited 10 Jan '20

10 Jan '20

alsa-project/alsa-lib issue #22 was edited from Bertware: I am using a Raspberry Pi 1 B+ with a Hifiberry DAC+ADC Pro HAT. This HAT contains a PCM5122 Stereo DAC and an ADC. Now to the issue: The ADC has parameters such as `VINR1[SE]` and `VINR2[SE] + VINR1[SE]`. When using `alsactl store` or`alsactl restore`, restoring will fail due to an invalid file. `ALSA lib conf.c:1887:(_snd_config_load_with_include) _toplevel_:354:30:Unexpected char` This error points to the parameter `VINR1[SE]`, which is not surrounded by quotes. The other parameters, which also contain other special characters such as spaces, are surrounded by quotes. An excerpt of the file is included below. This clearly illustrates which names were encapsulated between quotes. ``` control.28 { iface MIXER name 'ADC Right Capture Source' value VINR1[SE] comment { access 'read write' type ENUMERATED count 1 item.0 'No Select' item.1 VINR1[SE] item.2 VINR2[SE] item.3 'VINR2[SE] + VINR1[SE]' item.4 VINR3[SE] item.5 'VINR3[SE] + VINR1[SE]' item.6 'VINR3[SE] + VINR2[SE]' item.7 'VINR3[SE] + VINR2[SE] + VINR1[SE]' item.8 VINR4[SE] item.9 'VINR4[SE] + VINR1[SE]' item.10 'VINR4[SE] + VINR2[SE]' item.11 'VINR4[SE] + VINR2[SE] + VINR1[SE]' item.12 'VINR4[SE] + VINR3[SE]' item.13 'VINR4[SE] + VINR3[SE] + VINR1[SE]' item.14 'VINR4[SE] + VINR3[SE] + VINR2[SE]' item.15 'VINR4[SE] + VINR3[SE] + VINR2[SE] + VINR1[SE]' item.16 '{VIN2P, VIN2M}[DIFF]' item.17 '{VIN3P, VIN3M}[DIFF]' item.18 '{VIN2P, VIN2M}[DIFF] + {VIN3P, VIN3M}[DIFF]' } } ``` Creation of the output data and writing it to a file is handled by the save_state method in alsactl: https://github.com/alsa-project/alsa-utils/blob/master/alsactl/state.c#L1539 This code in turn calls snd_config_save in the alsa-lib project: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… Then further to string_print: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… And then in this loop which decides which characters need to be quoted: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… *Proposed solution* The characters `[` and `]` should be a reason to put text between quotes. These characters should be added to the switch statement in print_string here: https://github.com/alsa-project/alsa-lib/blob/b20b400e2f598c86abaf697d66396… Regards, Bert Issue URL : https://github.com/alsa-project/alsa-lib/issues/22 Repository URL: https://github.com/alsa-project/alsa-lib

1 0