From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 3/3] M-Audio FTU: Add effect controls Date: Mon, 23 Apr 2012 11:52:20 +0200 Message-ID: References: <1335172568-13502-1-git-send-email-linuxaudio@showlabor.de> <1335172568-13502-4-git-send-email-linuxaudio@showlabor.de> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1335172568-13502-4-git-send-email-linuxaudio@showlabor.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Felix Homann Cc: alsa-devel@alsa-project.org, patch@alsa-project.org, mark@pogo.org.uk List-Id: alsa-devel@alsa-project.org At Mon, 23 Apr 2012 11:16:08 +0200, Felix Homann wrote: > > > Signed-off-by: Felix Homann Again, a bit more explanations please. > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c > index 596744f..be46657 100644 > --- a/sound/usb/mixer.c > +++ b/sound/usb/mixer.c > @@ -775,6 +775,24 @@ static void volume_control_quirks(struct usb_mixer_elem_info *cval, > struct snd_kcontrol *kctl) > { > switch (cval->mixer->chip->usb_id) { > + case USB_ID(0x0763, 0x2081): /* M-Audio Fast Track Ultra 8R */ > + case USB_ID(0x0763, 0x2080): /* M-Audio Fast Track Ultra */ > + if ((strcmp(kctl->id.name, "Effect Duration") == 0)) { > + snd_printk(KERN_INFO > + "set quirk for FTU Effect Duration\n"); Put a prefix in the info print to be identified properly. snd_printk() might be just printk() when CONFIG_SND_VERBOSE_PRINTK isn't set. > + cval->min = 0x0000; > + cval->max = 0x7f00; > + cval->res = 0x0100; > + break; > + } > + if ((strcmp(kctl->id.name, "Effect Volume") == 0) | Use the logical OR '||'. Also the parenthesis is superfluous. > + (strcmp(kctl->id.name, "Effect Feedback Volume") == 0)) { > + snd_printk(KERN_INFO > + "set quirks for FTU Effect Feedback/Volume\n"); > + cval->min = 0x00; > + cval->max = 0x7f; > + break; > + } > case USB_ID(0x0471, 0x0101): > case USB_ID(0x0471, 0x0104): > case USB_ID(0x0471, 0x0105): > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > index 5b52275..588fe9c 100644 > --- a/sound/usb/mixer_quirks.c > +++ b/sound/usb/mixer_quirks.c > @@ -58,7 +58,7 @@ static void usb_mixer_elem_free(struct snd_kcontrol *kctl) > * Since there doesn't seem to be a devices that needs a multichannel > * version, we keep it mono for simplicity. > */ > -static int snd_create_standard_mono_ctl(struct usb_mixer_interface *mixer, > +static int snd_create_std_mono_ctl(struct usb_mixer_interface *mixer, > unsigned int unitid, > unsigned int control, > unsigned int cmask, As already mentioned, rename should have been done in the first patch. > @@ -577,6 +577,302 @@ static int snd_nativeinstruments_create_mixer(struct usb_mixer_interface *mixer, > > /* M-Audio FastTrack Ultra quirks */ > > +/* FTU Effect switch */ > +struct snd_maudio_ftu_effect_switch_private_value { Well... The length of a name is a matter of taste, but it looks unnecessarily too long to me, as it appears in the casts. > + struct usb_mixer_interface *mixer; > + int cached_value; > + int is_cached; > +}; > + > + > +static int snd_maudio_ftu_effect_switch_info(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_info *uinfo) > +{ > + static char *texts[8] = {"Room 1", > + "Room 2", > + "Room 3", > + "Hall 1", > + "Hall 2", > + "Plate", > + "Delay", > + "Echo" > + }; > + > + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; > + uinfo->count = 1; > + uinfo->value.enumerated.items = 8; > + if (uinfo->value.enumerated.item > 7) > + uinfo->value.enumerated.item = 7; > + strcpy(uinfo->value.enumerated.name, > + texts[uinfo->value.enumerated.item]); > + > + return 0; > +} > + > +static int snd_maudio_ftu_effect_switch_get(struct snd_kcontrol *kctl, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_usb_audio *chip; > + struct usb_mixer_interface *mixer; > + struct snd_maudio_ftu_effect_switch_private_value *pval; > + int err, id, validx, val_len, val_type; > + unsigned char value[2]; > + > + pval = (struct snd_maudio_ftu_effect_switch_private_value *) > + kctl->private_value; > + > + if (pval->is_cached) { > + ucontrol->value.enumerated.item[0] = pval->cached_value; > + return 0; > + } > + > + mixer = (struct usb_mixer_interface *) pval->mixer; > + if (mixer == NULL) { > + snd_printd(KERN_ERR "mixer == NULL"); Use snd_BUG_ON() in such a case. > + return -1; > + } > + > + chip = (struct snd_usb_audio *) mixer->chip; > + if (chip == NULL) { > + snd_printd(KERN_ERR "chip == NULL"); Ditto. > + return -1; > + } > + > + id = 6; > + validx = 1; > + val_type = USB_MIXER_S16; > + > + value[0] = 0x00; > + value[1] = 0x00; > + > + val_len = 2; > + > + err = snd_usb_ctl_msg(chip->dev, > + usb_rcvctrlpipe(chip->dev, 0), UAC_GET_CUR, > + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN, > + validx << 8, snd_usb_ctrl_intf(chip) | (id << 8), > + value, val_len); > + if (err < 0) > + return -1; > + > + ucontrol->value.enumerated.item[0] = value[0]; > + pval->cached_value = value[0]; > + pval->is_cached = 1; > + > + return 0; > +} > + > +static int snd_maudio_ftu_effect_switch_put(struct snd_kcontrol *kctl, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_usb_audio *chip; > + struct snd_maudio_ftu_effect_switch_private_value *pval; > + > + struct usb_mixer_interface *mixer; > + int changed, cur_val, err, id, new_val, validx, val_len, val_type; > + unsigned char value[2]; > + > + changed = 0; > + id = 6; > + validx = 1; > + val_type = USB_MIXER_S16; > + > + val_len = 2; > + pval = (struct snd_maudio_ftu_effect_switch_private_value *) > + kctl->private_value; > + cur_val = pval->cached_value; > + new_val = ucontrol->value.enumerated.item[0]; > + > + mixer = (struct usb_mixer_interface *) pval->mixer; > + if (mixer == NULL) { > + snd_printd(KERN_ERR "mixer == NULL"); Ditto. > + return -1; > + } > + > + chip = (struct snd_usb_audio *) mixer->chip; > + if (chip == NULL) { > + snd_printd(KERN_ERR "chip == NULL"); Ditto. > + return -1; > + } > + > + if (!pval->is_cached) { > + /* Read current value */ > + err = snd_usb_ctl_msg(chip->dev, > + usb_rcvctrlpipe(chip->dev, 0), UAC_GET_CUR, > + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN, > + validx << 8, snd_usb_ctrl_intf(chip) | (id << 8), > + value, val_len); > + if (err < 0) > + return -1; > + > + 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; > + err = snd_usb_ctl_msg(chip->dev, > + usb_sndctrlpipe(chip->dev, 0), UAC_SET_CUR, > + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT, > + validx << 8, snd_usb_ctrl_intf(chip) | (id << 8), > + value, val_len); > + if (err < 0) > + return -1; > + > + pval->cached_value = new_val; > + pval->is_cached = 1; > + changed = 1; > + } > + > + return changed; > +} > + > +static int snd_maudio_ftu_create_effect_switch(struct usb_mixer_interface *mixer) > +{ > + struct snd_kcontrol_new template = { > + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > + .name = "Effect Program Switch", > + .index = 0, > + .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, > + .info = snd_maudio_ftu_effect_switch_info, > + .get = snd_maudio_ftu_effect_switch_get, > + .put = snd_maudio_ftu_effect_switch_put > + }; Use static. > + > + int err; > + struct snd_kcontrol *kctl; > + struct snd_maudio_ftu_effect_switch_private_value *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; > + kctl = snd_ctl_new1(&template, mixer->chip); > + if (!kctl) { > + kfree(pval); > + return -ENOMEM; > + } > + > + err = snd_ctl_add(mixer->chip->card, kctl); > + if (err < 0) > + return err; > + > + return 0; > +} > + > +static int snd_maudio_ftu_create_effect_volume_ctl(struct usb_mixer_interface *mixer) > +{ > + unsigned int id, control, cmask; > + int val_type; > + > + char name[] = "Effect Volume"; Use static. > + > + id = 6; > + val_type = USB_MIXER_U8; > + control = 2; > + cmask = 0; Use const. > + > + return snd_create_std_mono_ctl(mixer, id, control, cmask, val_type, > + name, snd_usb_mixer_vol_tlv); > +} > + > +static int snd_maudio_ftu_create_effect_duration_ctl(struct usb_mixer_interface *mixer) > +{ > + unsigned int id, control, cmask; > + int val_type; > + > + char name[] = "Effect Duration"; > + > + id = 6; > + val_type = USB_MIXER_S16; > + control = 3; > + cmask = 0; Use const. > + > + return snd_create_std_mono_ctl(mixer, id, control, cmask, val_type, > + name, snd_usb_mixer_vol_tlv); > +} > + > +static int snd_maudio_ftu_create_effect_feedback_ctl(struct usb_mixer_interface *mixer) > +{ > + unsigned int id, control, cmask; > + int val_type; > + > + char name[] = "Effect Feedback Volume"; > + > + id = 6; > + val_type = USB_MIXER_U8; > + control = 4; > + cmask = 0; Ditto. > + > + return snd_create_std_mono_ctl(mixer, id, control, cmask, val_type, > + name, NULL); > +} > + > +static int snd_maudio_ftu_create_effect_return_ctls(struct usb_mixer_interface *mixer) > +{ > + unsigned int id, control, cmask; > + int err, val_type, ch; > + char name[48]; > + > + id = 7; > + val_type = USB_MIXER_S16; > + control = 2; > + > + for (ch = 0; ch < 4; ++ch) { > + cmask = 1 << ch; > + snprintf(name, sizeof(name), > + "Effect Return %d Volume", ch + 1); > + err = snd_create_std_mono_ctl(mixer, id, control, > + cmask, val_type, name, > + snd_usb_mixer_vol_tlv); > + if (err < 0) > + return err; > + } > + > + return 0; > +} > + > +static int snd_maudio_ftu_create_effect_send_ctls(struct usb_mixer_interface *mixer) > +{ > + unsigned int id, control, cmask; > + int err, val_type, ch; > + char name[48]; > + > + id = 5; > + val_type = USB_MIXER_S16; > + control = 9; > + > + for (ch = 0; ch < 8; ++ch) { > + cmask = 1 << ch; > + snprintf(name, sizeof(name), > + "Effect Send AIn%d Volume", ch + 1); > + err = snd_create_std_mono_ctl(mixer, id, control, cmask, > + val_type, name, > + snd_usb_mixer_vol_tlv); > + if (err < 0) > + return err; > + } > + for (ch = 8; ch < 16; ++ch) { > + cmask = 1 << ch; > + snprintf(name, sizeof(name), > + "Effect Send DIn%d Volume", ch - 7); > + err = snd_create_std_mono_ctl(mixer, id, control, cmask, > + val_type, name, > + snd_usb_mixer_vol_tlv); > + if (err < 0) > + return err; > + } > + return 0; > +} > + > + > /* Create a volume control for FTU devices*/ > static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer) > { > @@ -624,10 +920,33 @@ static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer) > if (err < 0) > return err; > > + err = snd_maudio_ftu_create_effect_volume_ctl(mixer); > + if (err < 0) > + return err; > + > + err = snd_maudio_ftu_create_effect_duration_ctl(mixer); > + if (err < 0) > + return err; > + > + err = snd_maudio_ftu_create_effect_feedback_ctl(mixer); > + if (err < 0) > + return err; > + > + err = snd_maudio_ftu_create_effect_return_ctls(mixer); > + if (err < 0) > + return err; > + > + err = snd_maudio_ftu_create_effect_send_ctls(mixer); > + if (err < 0) > + return err; > + > + err = snd_maudio_ftu_create_effect_switch(mixer); > + if (err < 0) > + return err; > + > return 0; > } > > - > /* > * Create mixer for Electrix Ebox-44 > * > @@ -638,17 +957,26 @@ static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer) > > static int snd_ebox44_create_mixer(struct usb_mixer_interface *mixer) > { > - snd_create_std_mono_ctl(mixer, 4, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Headphone Playback Switch", NULL); > - snd_create_std_mono_ctl(mixer, 4, 2, 0x1, USB_MIXER_S16, "Headphone A Mix Playback Volume", NULL); > - snd_create_std_mono_ctl(mixer, 4, 2, 0x2, USB_MIXER_S16, "Headphone B Mix Playback Volume", NULL); > - > - snd_create_std_mono_ctl(mixer, 7, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Output Playback Switch", NULL); > - snd_create_std_mono_ctl(mixer, 7, 2, 0x1, USB_MIXER_S16, "Output A Playback Volume", NULL); > - snd_create_std_mono_ctl(mixer, 7, 2, 0x2, USB_MIXER_S16, "Output B Playback Volume", NULL); > - > - snd_create_std_mono_ctl(mixer, 10, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Input Capture Switch", NULL); > - snd_create_std_mono_ctl(mixer, 10, 2, 0x1, USB_MIXER_S16, "Input A Capture Volume", NULL); > - snd_create_std_mono_ctl(mixer, 10, 2, 0x2, USB_MIXER_S16, "Input B Capture Volume", NULL); > + snd_create_std_mono_ctl(mixer, 4, 1, 0x0, USB_MIXER_INV_BOOLEAN, > + "Headphone Playback Switch", NULL); > + snd_create_std_mono_ctl(mixer, 4, 2, 0x1, USB_MIXER_S16, > + "Headphone A Mix Playback Volume", NULL); > + snd_create_std_mono_ctl(mixer, 4, 2, 0x2, USB_MIXER_S16, > + "Headphone B Mix Playback Volume", NULL); > + > + snd_create_std_mono_ctl(mixer, 7, 1, 0x0, USB_MIXER_INV_BOOLEAN, > + "Output Playback Switch", NULL); > + snd_create_std_mono_ctl(mixer, 7, 2, 0x1, USB_MIXER_S16, > + "Output A Playback Volume", NULL); > + snd_create_std_mono_ctl(mixer, 7, 2, 0x2, USB_MIXER_S16, > + "Output B Playback Volume", NULL); > + > + snd_create_std_mono_ctl(mixer, 10, 1, 0x0, USB_MIXER_INV_BOOLEAN, > + "Input Capture Switch", NULL); > + snd_create_std_mono_ctl(mixer, 10, 2, 0x1, USB_MIXER_S16, > + "Input A Capture Volume", NULL); > + snd_create_std_mono_ctl(mixer, 10, 2, 0x2, USB_MIXER_S16, > + "Input B Capture Volume", NULL); These should have been applied in the first patch. thanks, Takashi