[alsa-devel] [PATCH 1/2] snd-usb-audio: Add mixer control for Digidesign Mbox 1 clock source
Takashi Iwai
tiwai at suse.de
Thu Nov 6 15:12:04 CET 2014
At Tue, 4 Nov 2014 11:06:36 +1100,
Damien Zammit wrote:
>
> Signed-off-by: Damien Zammit <damien at zamaudio.com>
> ---
> sound/usb/mixer_maps.c | 9 +++
> sound/usb/mixer_quirks.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 186 insertions(+)
>
> diff --git a/sound/usb/mixer_maps.c b/sound/usb/mixer_maps.c
> index d1d72ff..9ee3fc2 100644
> --- a/sound/usb/mixer_maps.c
> +++ b/sound/usb/mixer_maps.c
> @@ -179,6 +179,11 @@ static struct usbmix_name_map audigy2nx_map[] = {
> { 0 } /* terminator */
> };
>
> +static struct usbmix_name_map mbox1_map[] = {
> + { 1, "Control" },
"Control" sounds too ambiguous.
> + { 0 } /* terminator */
> +};
> +
> static struct usbmix_selector_map c400_selectors[] = {
> {
> .id = 0x80,
> @@ -416,6 +421,10 @@ static struct usbmix_ctl_map usbmix_ctl_maps[] = {
> .map = aureon_51_2_map,
> },
> {
> + .id = USB_ID(0x0dba, 0x1000),
> + .map = mbox1_map,
> + },
> + {
> .id = USB_ID(0x13e5, 0x0001),
> .map = scratch_live_map,
> .ignore_ctl_error = 1,
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index 3980bf5..716c32c 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -565,6 +565,176 @@ static int snd_xonar_u1_controls_create(struct usb_mixer_interface *mixer)
> return 0;
> }
>
> +/* Digidesign Mbox 1 clock source switch (internal/spdif) */
> +
> +struct snd_mbox1_switch_priv_val {
> + struct usb_mixer_interface *mixer;
> + int cached_value;
> + int is_cached;
Use bool instead of int. Also true/false for 1/0.
> +};
> +
> +static int snd_mbox1_switch_get(struct snd_kcontrol *kctl,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_mbox1_switch_priv_val *pval;
> + unsigned char value;
> +
> + value = 0x00;
> +
> + pval = (struct snd_mbox1_switch_priv_val *)
> + kctl->private_value;
> +
> + if (pval->is_cached) {
> + ucontrol->value.enumerated.item[0] = pval->cached_value;
> + return 0;
> + }
> +
> + ucontrol->value.enumerated.item[0] = value;
> + pval->cached_value = value;
> + pval->is_cached = 1;
> +
> + return 0;
> +}
> +
> +static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_usb_audio *chip;
> + struct snd_mbox1_switch_priv_val *pval;
> +
> + struct usb_mixer_interface *mixer;
> + int changed, cur_val, err, new_val;
> + unsigned char value[2];
> + unsigned char buff[3];
> +
> + changed = 0;
> + value[0] = 0x00;
> + value[1] = 0x00;
> +
> + pval = (struct snd_mbox1_switch_priv_val *)
> + kctl->private_value;
> + cur_val = pval->cached_value;
> + new_val = ucontrol->value.enumerated.item[0];
> +
> + mixer = (struct usb_mixer_interface *) pval->mixer;
Superfluous cast.
> + if (snd_BUG_ON(!mixer))
> + return -EINVAL;
> +
> + chip = (struct snd_usb_audio *) mixer->chip;
Ditto.
> + if (snd_BUG_ON(!chip))
> + return -EINVAL;
> +
> + if (!pval->is_cached) {
> + cur_val = value[0];
> + pval->cached_value = cur_val;
> + pval->is_cached = 1;
> + }
> + /* update value if needed */
> + if (cur_val != new_val) {
> + value[0] = new_val;
> + value[1] = 0;
> + down_read(&mixer->chip->shutdown_rwsem);
You can replace mixer->chip with chip (in below too).
> + if (mixer->chip->shutdown) {
> + err = -ENODEV;
> + } else {
> + err = snd_usb_ctl_msg(mixer->chip->dev,
> + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
> + USB_DIR_IN |
> + USB_TYPE_CLASS |
> + USB_RECIP_INTERFACE, 0x00, 0x500, buff, 1);
> + if (err < 0)
> + return err;
> + err = snd_usb_ctl_msg(mixer->chip->dev,
> + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
> + USB_DIR_IN |
> + USB_TYPE_CLASS |
> + USB_RECIP_ENDPOINT, 0x100, 0x81, buff, 3);
> + if (err < 0)
> + return err;
What do these two messages and why needed?
> + if (new_val == 0) {
> + buff[0] = 0x80;
> + buff[1] = 0xbb;
> + buff[2] = 0x00;
> + } else {
> + buff[0] = buff[1] = buff[2] = 0;
> + }
> + err = snd_usb_ctl_msg(mixer->chip->dev,
> + usb_sndctrlpipe(mixer->chip->dev, 0), 0x1,
> + USB_TYPE_CLASS |
> + USB_RECIP_ENDPOINT, 0x100, 0x81, buff, 3);
> + if (err < 0)
> + return err;
> + err = snd_usb_ctl_msg(mixer->chip->dev,
> + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
> + USB_DIR_IN |
> + USB_TYPE_CLASS |
> + USB_RECIP_ENDPOINT, 0x100, 0x81, buff, 3);
> + if (err < 0)
> + return err;
> + err = snd_usb_ctl_msg(mixer->chip->dev,
> + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
> + USB_DIR_IN |
> + USB_TYPE_CLASS |
> + USB_RECIP_ENDPOINT, 0x100, 0x2, buff, 3);
> + if (err < 0)
> + return err;
Ditto.
> + }
> + up_read(&mixer->chip->shutdown_rwsem);
> + if (err < 0)
> + return err;
> +
> + pval->cached_value = new_val;
> + pval->is_cached = 1;
> + changed = 1;
> + }
> +
> + return changed;
> +}
> +
> +static int snd_mbox1_switch_info(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_info *uinfo)
> +{
> + static const char *const texts[2] = {
> + "Internal",
> + "S/PDIF"
> + };
> +
> + return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
> +}
> +
> +static int snd_mbox1_create_sync_switch(struct usb_mixer_interface *mixer)
> +{
> + static struct snd_kcontrol_new template = {
> + .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> + .name = "Clock Source",
> + .index = 0,
> + .access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> + .info = snd_mbox1_switch_info,
> + .get = snd_mbox1_switch_get,
> + .put = snd_mbox1_switch_put
> + };
> +
> + struct snd_kcontrol *kctl;
> + struct snd_mbox1_switch_priv_val *pval;
> +
> + pval = kzalloc(sizeof(*pval), GFP_KERNEL);
> + if (!pval)
> + return -ENOMEM;
> +
> + pval->cached_value = 0;
> + pval->is_cached = 0;
> + pval->mixer = mixer;
> +
> + template.private_value = (unsigned long) pval;
This is basically racy (although it wouldn't happen practically
much) since static variable isn't local, thus two concurrent accesses
overwrite with each other. Set kct->private_value instead of the
template below.
> + kctl = snd_ctl_new1(&template, mixer->chip);
> + if (!kctl) {
> + kfree(pval);
> + return -ENOMEM;
> + }
You need to set private_free. Otherwise pval will be leaked.
thanks,
Takashi
> +
> + return snd_ctl_add(mixer->chip->card, kctl);
> +}
> +
> /* Native Instruments device quirks */
>
> #define _MAKE_NI_CONTROL(bRequest,wIndex) ((bRequest) << 16 | (wIndex))
> @@ -1605,6 +1775,13 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
> snd_audigy2nx_proc_read);
> break;
>
> + /* Digidesign Mbox 1 */
> + case USB_ID(0x0dba, 0x1000):
> + err = snd_mbox1_create_sync_switch(mixer);
> + if (err < 0)
> + break;
> + break;
> +
> /* EMU0204 */
> case USB_ID(0x041e, 0x3f19):
> err = snd_emu0204_controls_create(mixer);
> --
> 1.9.1
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
More information about the Alsa-devel
mailing list