[PATCH 0/3] Alsa: USB: Improve M-Audio Fast Track Ultra mixer

[PATCH 0/3] Alsa: USB: Improve M-Audio Fast Track Ultra mixer

[Felix Homann]

This series of patches improves the mixer for Fast Track Ultra (8R)
devices by

	* adding TLV to existing controls
	* adding new controls for the effect section.

It unifies the control creation for FTU and Ebox-44 devices.

Mark, could you please look at mixer_quirks.c. Do we really need the range settings 
in snd_create_std_mono_ctl()? BTW, you might like to add some TLV callback to the
Ebox-44 controls as well.

Regards,

Felix

Felix Homann (3):
  Unify M-Audio Fast Track Ultra and Ebox-44 mixer quirks.
  Add TLV to M-Audio volume controls
  M-Audio FTU: Add effect controls

 sound/usb/mixer.c        |   23 +++
 sound/usb/mixer_quirks.c |  493 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 450 insertions(+), 66 deletions(-)

-- 
1.7.5.4

[PATCH 1/3] Unify M-Audio Fast Track Ultra and Ebox-44 mixer quirks.

[Felix Homann]

Allows for specifying a TLV callback.

Signed-off-by: Felix Homann <linuxaudio@showlabor.de>

diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index e2072ed..e486c51 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -42,6 +42,85 @@
 
 extern struct snd_kcontrol_new *snd_usb_feature_unit_ctl;
 
+/* private_free callback */
+static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
+{
+	kfree(kctl->private_data);
+	kctl->private_data = NULL;
+}
+
+/* This function allows for the creation of standard UAC controls.
+ * See the quirks for M-Audio FTUs or Ebox-44.
+ * If you don't want to set a TLV callback pass NULL.
+ * 
+ * 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,
+				unsigned int unitid,
+				unsigned int control,
+				unsigned int cmask,
+				int val_type,
+				const char *name,
+				snd_kcontrol_tlv_rw_t *tlv_callback)
+{
+	int err;
+	struct usb_mixer_elem_info *cval;
+	struct snd_kcontrol *kctl;
+
+	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
+	if (!cval)
+		return -ENOMEM;
+
+	cval->id = unitid;
+	cval->mixer = mixer;
+	cval->val_type = val_type;
+	cval->channels = 1;
+	cval->control = control;
+	cval->cmask = cmask;
+
+	/* FIXME: Do we need this?
+	 * The following values are for compatibility with
+	 * Ebox-44 mixer.
+	 * But the corresponding ebox-44 function says:
+	 *    "Volume controls will override these values"
+	 * 
+	 * These values don't have any effect at all for
+	 * M-Audio FTUs.
+	 * So I think, we can safely omit the range settings here.
+	 */
+	cval->min = 0;
+	cval->max = 1;
+	cval->res = 0;
+	cval->dBmin = 0;
+	cval->dBmax = 0;
+
+	/* Create control */
+	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
+	if (!kctl) {
+		kfree(cval);
+		return -ENOMEM;
+	}
+
+	/* Set name */
+	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
+	kctl->private_free = usb_mixer_elem_free;
+
+	/* set TLV */
+	if (tlv_callback != NULL) {
+		kctl->tlv.c = tlv_callback;
+		kctl->vd[0].access |=
+			SNDRV_CTL_ELEM_ACCESS_TLV_READ |
+			SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
+	}
+	/* Add control to mixer */
+	err = snd_usb_mixer_add_control(mixer, kctl);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 /*
  * Sound Blaster remote control configuration
  *
@@ -496,59 +575,37 @@ static int snd_nativeinstruments_create_mixer(struct usb_mixer_interface *mixer,
 
 /* M-Audio FastTrack Ultra quirks */
 
-/* private_free callback */
-static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
-{
-	kfree(kctl->private_data);
-	kctl->private_data = NULL;
-}
-
-static int snd_maudio_ftu_create_ctl(struct usb_mixer_interface *mixer,
-				     int in, int out, const char *name)
-{
-	struct usb_mixer_elem_info *cval;
-	struct snd_kcontrol *kctl;
-
-	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
-	if (!cval)
-		return -ENOMEM;
-
-	cval->id = 5;
-	cval->mixer = mixer;
-	cval->val_type = USB_MIXER_S16;
-	cval->channels = 1;
-	cval->control = out + 1;
-	cval->cmask = 1 << in;
-
-	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
-	if (!kctl) {
-		kfree(cval);
-		return -ENOMEM;
-	}
-
-	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
-	kctl->private_free = usb_mixer_elem_free;
-	return snd_usb_mixer_add_control(mixer, kctl);
-}
-
-static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
+/* Create a volume control for FTU devices*/
+static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer)
 {
 	char name[64];
-	int in, out, err;
+	unsigned int id, control, cmask;
+	int in, out, err, val_type;
+
+	id = 5;
+	val_type = USB_MIXER_S16;
 
 	for (out = 0; out < 8; out++) {
+		control = out + 1;
 		for (in = 0; in < 8; in++) {
+			cmask = 1 << in;
 			snprintf(name, sizeof(name),
-				 "AIn%d - Out%d Capture Volume", in  + 1, out + 1);
-			err = snd_maudio_ftu_create_ctl(mixer, in, out, name);
+				"AIn%d - Out%d Capture Volume",
+				in  + 1, out + 1);
+			err = snd_create_std_mono_ctl(mixer, id, control,
+							cmask, val_type, name,
+							NULL);
 			if (err < 0)
 				return err;
 		}
-
 		for (in = 8; in < 16; in++) {
+			cmask = 1 << in;
 			snprintf(name, sizeof(name),
-				 "DIn%d - Out%d Playback Volume", in - 7, out + 1);
-			err = snd_maudio_ftu_create_ctl(mixer, in, out, name);
+				"DIn%d - Out%d Playback Volume",
+				in - 7, out + 1);
+			err = snd_create_std_mono_ctl(mixer, id, control,
+							cmask, val_type, name,
+							NULL);
 			if (err < 0)
 				return err;
 		}
@@ -557,44 +614,18 @@ static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
 	return 0;
 }
 
-static int snd_ebox44_create_ctl(struct usb_mixer_interface *mixer,
-				int unitid, int control, int cmask,
-				int val_type, const char *name)
+static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
 {
-	struct usb_mixer_elem_info *cval;
-	struct snd_kcontrol *kctl;
-
-	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
-	if (!cval)
-		return -ENOMEM;
-
-	cval->id = unitid;
-	cval->mixer = mixer;
-
-	cval->val_type = val_type;
-	cval->channels = 1;
-	cval->control = control;
-	cval->cmask = cmask;
-
-	/* Volume controls will override these values */
-	cval->min = 0;
-	cval->max = 1;
-	cval->res = 0;
-
-	cval->dBmin = 0;
-	cval->dBmax = 0;
+	int err;
 
-	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
-	if (!kctl) {
-		kfree(cval);
-		return -ENOMEM;
-	}
+	err = snd_maudio_ftu_create_volume_ctls(mixer);
+	if (err < 0)
+		return err;
 
-	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
-	kctl->private_free = usb_mixer_elem_free;
-	return snd_usb_mixer_add_control(mixer, kctl);
+	return 0;
 }
 
+
 /*
  * Create mixer for Electrix Ebox-44
  *
@@ -605,17 +636,17 @@ static int snd_ebox44_create_ctl(struct usb_mixer_interface *mixer,
 
 static int snd_ebox44_create_mixer(struct usb_mixer_interface *mixer)
 {
-	snd_ebox44_create_ctl(mixer, 4, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Headphone Playback Switch");
-	snd_ebox44_create_ctl(mixer, 4, 2, 0x1, USB_MIXER_S16, "Headphone A Mix Playback Volume");
-	snd_ebox44_create_ctl(mixer, 4, 2, 0x2, USB_MIXER_S16, "Headphone B Mix Playback Volume");
+	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_ebox44_create_ctl(mixer, 7, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Output Playback Switch");
-	snd_ebox44_create_ctl(mixer, 7, 2, 0x1, USB_MIXER_S16, "Output A Playback Volume");
-	snd_ebox44_create_ctl(mixer, 7, 2, 0x2, USB_MIXER_S16, "Output B Playback Volume");
+	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_ebox44_create_ctl(mixer, 10, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Input Capture Switch");
-	snd_ebox44_create_ctl(mixer, 10, 2, 0x1, USB_MIXER_S16, "Input A Capture Volume");
-	snd_ebox44_create_ctl(mixer, 10, 2, 0x2, USB_MIXER_S16, "Input B Capture Volume");
+	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);
 
 	return 0;
 }
-- 
1.7.5.4

Re: [PATCH 1/3] Unify M-Audio Fast Track Ultra and Ebox-44 mixer quirks.

[Takashi Iwai]

At Mon, 23 Apr 2012 11:16:06 +0200,
Felix Homann wrote:
> 
> Allows for specifying a TLV callback.

Too little information as the patch changelog.
Please be more specific what and how is implemented there.

> 
> Signed-off-by: Felix Homann <linuxaudio@showlabor.de>
> 
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index e2072ed..e486c51 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -42,6 +42,85 @@
>  
>  extern struct snd_kcontrol_new *snd_usb_feature_unit_ctl;
>  
> +/* private_free callback */
> +static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
> +{
> +	kfree(kctl->private_data);
> +	kctl->private_data = NULL;
> +}
> +
> +/* This function allows for the creation of standard UAC controls.
> + * See the quirks for M-Audio FTUs or Ebox-44.
> + * If you don't want to set a TLV callback pass NULL.
> + * 
> + * 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,
> +				unsigned int unitid,
> +				unsigned int control,
> +				unsigned int cmask,
> +				int val_type,
> +				const char *name,
> +				snd_kcontrol_tlv_rw_t *tlv_callback)

This should be snd_create_std_mono_ctl(), no?
Each patch should be able to be built properly.  Otherwise it breaks
the bisection.


> +{
> +	int err;
> +	struct usb_mixer_elem_info *cval;
> +	struct snd_kcontrol *kctl;
> +
> +	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
> +	if (!cval)
> +		return -ENOMEM;
> +
> +	cval->id = unitid;
> +	cval->mixer = mixer;
> +	cval->val_type = val_type;
> +	cval->channels = 1;
> +	cval->control = control;
> +	cval->cmask = cmask;
> +
> +	/* FIXME: Do we need this?
> +	 * The following values are for compatibility with
> +	 * Ebox-44 mixer.
> +	 * But the corresponding ebox-44 function says:
> +	 *    "Volume controls will override these values"
> +	 * 
> +	 * These values don't have any effect at all for
> +	 * M-Audio FTUs.
> +	 * So I think, we can safely omit the range settings here.
> +	 */
> +	cval->min = 0;
> +	cval->max = 1;
> +	cval->res = 0;
> +	cval->dBmin = 0;
> +	cval->dBmax = 0;
> +
> +	/* Create control */
> +	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
> +	if (!kctl) {
> +		kfree(cval);
> +		return -ENOMEM;
> +	}
> +
> +	/* Set name */
> +	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
> +	kctl->private_free = usb_mixer_elem_free;
> +
> +	/* set TLV */
> +	if (tlv_callback != NULL) {

Should be "if (!tlv_callback)"

> +		kctl->tlv.c = tlv_callback;
> +		kctl->vd[0].access |=
> +			SNDRV_CTL_ELEM_ACCESS_TLV_READ |
> +			SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
> +	}
> +	/* Add control to mixer */
> +	err = snd_usb_mixer_add_control(mixer, kctl);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +
>  /*
>   * Sound Blaster remote control configuration
>   *
> @@ -496,59 +575,37 @@ static int snd_nativeinstruments_create_mixer(struct usb_mixer_interface *mixer,
>  
>  /* M-Audio FastTrack Ultra quirks */
>  
> -/* private_free callback */
> -static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
> -{
> -	kfree(kctl->private_data);
> -	kctl->private_data = NULL;
> -}
> -
> -static int snd_maudio_ftu_create_ctl(struct usb_mixer_interface *mixer,
> -				     int in, int out, const char *name)
> -{
> -	struct usb_mixer_elem_info *cval;
> -	struct snd_kcontrol *kctl;
> -
> -	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
> -	if (!cval)
> -		return -ENOMEM;
> -
> -	cval->id = 5;
> -	cval->mixer = mixer;
> -	cval->val_type = USB_MIXER_S16;
> -	cval->channels = 1;
> -	cval->control = out + 1;
> -	cval->cmask = 1 << in;
> -
> -	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
> -	if (!kctl) {
> -		kfree(cval);
> -		return -ENOMEM;
> -	}
> -
> -	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
> -	kctl->private_free = usb_mixer_elem_free;
> -	return snd_usb_mixer_add_control(mixer, kctl);
> -}
> -
> -static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
> +/* Create a volume control for FTU devices*/
> +static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer)
>  {
>  	char name[64];
> -	int in, out, err;
> +	unsigned int id, control, cmask;
> +	int in, out, err, val_type;
> +
> +	id = 5;
> +	val_type = USB_MIXER_S16;

Give const.  I don't mind use such styles instead of define or
hard-coded in the function calls, but then it should be marked as
const properly.


thanks,

Takashi


>  
>  	for (out = 0; out < 8; out++) {
> +		control = out + 1;
>  		for (in = 0; in < 8; in++) {
> +			cmask = 1 << in;
>  			snprintf(name, sizeof(name),
> -				 "AIn%d - Out%d Capture Volume", in  + 1, out + 1);
> -			err = snd_maudio_ftu_create_ctl(mixer, in, out, name);
> +				"AIn%d - Out%d Capture Volume",
> +				in  + 1, out + 1);
> +			err = snd_create_std_mono_ctl(mixer, id, control,
> +							cmask, val_type, name,
> +							NULL);
>  			if (err < 0)
>  				return err;
>  		}
> -
>  		for (in = 8; in < 16; in++) {
> +			cmask = 1 << in;
>  			snprintf(name, sizeof(name),
> -				 "DIn%d - Out%d Playback Volume", in - 7, out + 1);
> -			err = snd_maudio_ftu_create_ctl(mixer, in, out, name);
> +				"DIn%d - Out%d Playback Volume",
> +				in - 7, out + 1);
> +			err = snd_create_std_mono_ctl(mixer, id, control,
> +							cmask, val_type, name,
> +							NULL);
>  			if (err < 0)
>  				return err;
>  		}
> @@ -557,44 +614,18 @@ static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
>  	return 0;
>  }
>  
> -static int snd_ebox44_create_ctl(struct usb_mixer_interface *mixer,
> -				int unitid, int control, int cmask,
> -				int val_type, const char *name)
> +static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
>  {
> -	struct usb_mixer_elem_info *cval;
> -	struct snd_kcontrol *kctl;
> -
> -	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
> -	if (!cval)
> -		return -ENOMEM;
> -
> -	cval->id = unitid;
> -	cval->mixer = mixer;
> -
> -	cval->val_type = val_type;
> -	cval->channels = 1;
> -	cval->control = control;
> -	cval->cmask = cmask;
> -
> -	/* Volume controls will override these values */
> -	cval->min = 0;
> -	cval->max = 1;
> -	cval->res = 0;
> -
> -	cval->dBmin = 0;
> -	cval->dBmax = 0;
> +	int err;
>  
> -	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
> -	if (!kctl) {
> -		kfree(cval);
> -		return -ENOMEM;
> -	}
> +	err = snd_maudio_ftu_create_volume_ctls(mixer);
> +	if (err < 0)
> +		return err;
>  
> -	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
> -	kctl->private_free = usb_mixer_elem_free;
> -	return snd_usb_mixer_add_control(mixer, kctl);
> +	return 0;
>  }
>  
> +
>  /*
>   * Create mixer for Electrix Ebox-44
>   *
> @@ -605,17 +636,17 @@ static int snd_ebox44_create_ctl(struct usb_mixer_interface *mixer,
>  
>  static int snd_ebox44_create_mixer(struct usb_mixer_interface *mixer)
>  {
> -	snd_ebox44_create_ctl(mixer, 4, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Headphone Playback Switch");
> -	snd_ebox44_create_ctl(mixer, 4, 2, 0x1, USB_MIXER_S16, "Headphone A Mix Playback Volume");
> -	snd_ebox44_create_ctl(mixer, 4, 2, 0x2, USB_MIXER_S16, "Headphone B Mix Playback Volume");
> +	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_ebox44_create_ctl(mixer, 7, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Output Playback Switch");
> -	snd_ebox44_create_ctl(mixer, 7, 2, 0x1, USB_MIXER_S16, "Output A Playback Volume");
> -	snd_ebox44_create_ctl(mixer, 7, 2, 0x2, USB_MIXER_S16, "Output B Playback Volume");
> +	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_ebox44_create_ctl(mixer, 10, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Input Capture Switch");
> -	snd_ebox44_create_ctl(mixer, 10, 2, 0x1, USB_MIXER_S16, "Input A Capture Volume");
> -	snd_ebox44_create_ctl(mixer, 10, 2, 0x2, USB_MIXER_S16, "Input B Capture Volume");
> +	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);
>  
>  	return 0;
>  }
> -- 
> 1.7.5.4
> 

Re: [PATCH 1/3] Unify M-Audio Fast Track Ultra and Ebox-44 mixer quirks.

[Felix Homann]

2012/4/23 Takashi Iwai <tiwai@suse.de>:
>> Allows for specifying a TLV callback.
>
> Too little information as the patch changelog.
> Please be more specific what and how is implemented there.

OK.

> This should be snd_create_std_mono_ctl(), no?

Yes, I wasn't aware that I had only fixed this in a later commit.

> Each patch should be able to be built properly.  Otherwise it breaks
> the bisection.

Yes, that makes sense.


>> +/* Create a volume control for FTU devices*/
>> +static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer)
>>  {
>>       char name[64];
>> -     int in, out, err;
>> +     unsigned int id, control, cmask;
>> +     int in, out, err, val_type;
>> +
>> +     id = 5;
>> +     val_type = USB_MIXER_S16;
>
> Give const.  I don't mind use such styles instead of define or
> hard-coded in the function calls, but then it should be marked as
> const properly.

I don't mind to use a define here. Just tell me what you prefer.

I'll prepare a coreccted patch set soon.

Regards,

Felix

Re: [PATCH 1/3] Unify M-Audio Fast Track Ultra and Ebox-44 mixer quirks.

[Takashi Iwai]

At Mon, 23 Apr 2012 11:57:32 +0200,
Felix Homann wrote:
> 
> 2012/4/23 Takashi Iwai <tiwai@suse.de>:
> >> Allows for specifying a TLV callback.
> >
> > Too little information as the patch changelog.
> > Please be more specific what and how is implemented there.
> 
> OK.
> 
> > This should be snd_create_std_mono_ctl(), no?
> 
> Yes, I wasn't aware that I had only fixed this in a later commit.
> 
> > Each patch should be able to be built properly.  Otherwise it breaks
> > the bisection.
> 
> Yes, that makes sense.
> 
> 
> >> +/* Create a volume control for FTU devices*/
> >> +static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer)
> >>  {
> >>       char name[64];
> >> -     int in, out, err;
> >> +     unsigned int id, control, cmask;
> >> +     int in, out, err, val_type;
> >> +
> >> +     id = 5;
> >> +     val_type = USB_MIXER_S16;
> >
> > Give const.  I don't mind use such styles instead of define or
> > hard-coded in the function calls, but then it should be marked as
> > const properly.
> 
> I don't mind to use a define here. Just tell me what you prefer.

Well, I don't mind either way.  The point is that you should add const
if you keep that style.


> I'll prepare a coreccted patch set soon.

Thanks!


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/3] Add TLV to M-Audio volume controls

[Felix Homann]

Signed-off-by: Felix Homann <linuxaudio@showlabor.de>

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index c374c72..596744f 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -501,6 +501,11 @@ static int mixer_vol_tlv(struct snd_kcontrol *kcontrol, int op_flag,
 	return 0;
 }
 
+/* This symbol is exported in order to reuse it in mixer_quirks.c */
+int (*snd_usb_mixer_vol_tlv)(struct snd_kcontrol *kcontrol, int op_flag,
+			unsigned int size,
+			unsigned int __user *_tlv) = &mixer_vol_tlv;
+
 /*
  * parser routines begin here...
  */
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index e486c51..5b52275 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -41,6 +41,8 @@
 #include "helper.h"
 
 extern struct snd_kcontrol_new *snd_usb_feature_unit_ctl;
+extern int (*snd_usb_mixer_vol_tlv)(struct snd_kcontrol *kcontrol, int op_flag,
+				unsigned int size, unsigned int __user *_tlv);
 
 /* private_free callback */
 static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
@@ -594,7 +596,7 @@ static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer)
 				in  + 1, out + 1);
 			err = snd_create_std_mono_ctl(mixer, id, control,
 							cmask, val_type, name,
-							NULL);
+							snd_usb_mixer_vol_tlv);
 			if (err < 0)
 				return err;
 		}
@@ -605,7 +607,7 @@ static int snd_maudio_ftu_create_volume_ctls(struct usb_mixer_interface *mixer)
 				in - 7, out + 1);
 			err = snd_create_std_mono_ctl(mixer, id, control,
 							cmask, val_type, name,
-							NULL);
+							snd_usb_mixer_vol_tlv);
 			if (err < 0)
 				return err;
 		}
-- 
1.7.5.4

Re: [PATCH 2/3] Add TLV to M-Audio volume controls

[Takashi Iwai]

At Mon, 23 Apr 2012 11:16:07 +0200,
Felix Homann wrote:
> 
> 
> Signed-off-by: Felix Homann <linuxaudio@showlabor.de>
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index c374c72..596744f 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -501,6 +501,11 @@ static int mixer_vol_tlv(struct snd_kcontrol *kcontrol, int op_flag,
>  	return 0;
>  }
>  
> +/* This symbol is exported in order to reuse it in mixer_quirks.c */
> +int (*snd_usb_mixer_vol_tlv)(struct snd_kcontrol *kcontrol, int op_flag,
> +			unsigned int size,
> +			unsigned int __user *_tlv) = &mixer_vol_tlv;

Where is it defined...?


Takashi

[PATCH 3/3] M-Audio FTU: Add effect controls

[Felix Homann]

Signed-off-by: Felix Homann <linuxaudio@showlabor.de>

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");
+			cval->min = 0x0000;
+			cval->max = 0x7f00;
+			cval->res = 0x0100;
+			break;
+		}
+		if ((strcmp(kctl->id.name, "Effect Volume") == 0) |
+			(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,
@@ -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 {
+	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");
+		return -1;
+	}
+
+	chip = (struct snd_usb_audio *) mixer->chip;
+	if (chip == NULL) {
+		snd_printd(KERN_ERR "chip == NULL");
+		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");
+		return -1;
+	}
+
+	chip = (struct snd_usb_audio *) mixer->chip;
+	if (chip == NULL) {
+		snd_printd(KERN_ERR "chip == NULL");
+		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
+	};
+
+	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";
+
+	id = 6;
+	val_type = USB_MIXER_U8;
+	control = 2;
+	cmask = 0;
+
+	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;
+
+	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;
+
+	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);
 
 	return 0;
 }
-- 
1.7.5.4

Re: [PATCH 3/3] M-Audio FTU: Add effect controls

[Takashi Iwai]

At Mon, 23 Apr 2012 11:16:08 +0200,
Felix Homann wrote:
> 
> 
> Signed-off-by: Felix Homann <linuxaudio@showlabor.de>

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

Re: [PATCH 3/3] M-Audio FTU: Add effect controls

[Felix Homann]

Hi,

2012/4/23 Takashi Iwai <tiwai@suse.de>:
> At Mon, 23 Apr 2012 11:16:08 +0200,
> Felix Homann wrote:
>>
>>
>> Signed-off-by: Felix Homann <linuxaudio@showlabor.de>
>
> 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.

Hmm, again I just followed the examples of other volume quirks. What
kind of prefix would you like?

>
>
>> +                     cval->min = 0x0000;
>> +                     cval->max = 0x7f00;
>> +                     cval->res = 0x0100;
>> +                     break;
>> +             }
>> +             if ((strcmp(kctl->id.name, "Effect Volume") == 0) |
>
> Use the logical OR '||'.

Oh yes, stupid me...

> Also the parenthesis is superfluous.

I know. I put them there because checkpatch.pl complained. What's the
right way to cope with checkpatch.pl complains then?


>>
>>  /* 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.

It's not to my taste either. But it's in line with the existing ftu
quirks. I'm already preparing a patch for the M-Audio C-400 mixer.
What do you think of

* renaming the FTU specifiy functions from snd_maudio_ftu_* to snd_ftu_*
* renaming functions common to different M-Audio devices to snd_maudio_*
* specifically renaming snd_maudio_ftu_effect_switch_private_value to
snd_maudio_effect_switch_priv_val (well, it's just a bit shorter...)



>> +     mixer = (struct usb_mixer_interface *) pval->mixer;
>> +     if (mixer == NULL) {
>> +             snd_printd(KERN_ERR "mixer == NULL");
>
> Use snd_BUG_ON() in such a case.

What's that?


>> +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.

OK.


>> +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.

OK.

>
>> +
>> +     id = 6;
>> +     val_type = USB_MIXER_U8;
>> +     control = 2;
>> +     cmask = 0;
>
> Use const.

OK.

>> +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.

OK.


>> +                             "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.

OK. I wasn't aware of the extremely reasonable policy of not breaking
the build with a patch...

Sorry,

Felix

Re: [PATCH 3/3] M-Audio FTU: Add effect controls

[Takashi Iwai]

At Mon, 23 Apr 2012 12:19:39 +0200,
Felix Homann wrote:
> 
> Hi,
> 
> 2012/4/23 Takashi Iwai <tiwai@suse.de>:
> > At Mon, 23 Apr 2012 11:16:08 +0200,
> > Felix Homann wrote:
> >>
> >>
> >> Signed-off-by: Felix Homann <linuxaudio@showlabor.de>
> >
> > 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.
> 
> Hmm, again I just followed the examples of other volume quirks. What
> kind of prefix would you like?

Just like "usb-audio:" or something like that.

> >> +                     cval->min = 0x0000;
> >> +                     cval->max = 0x7f00;
> >> +                     cval->res = 0x0100;
> >> +                     break;
> >> +             }
> >> +             if ((strcmp(kctl->id.name, "Effect Volume") == 0) |
> >
> > Use the logical OR '||'.
> 
> Oh yes, stupid me...
> 
> > Also the parenthesis is superfluous.
> 
> I know. I put them there because checkpatch.pl complained. What's the
> right way to cope with checkpatch.pl complains then?

It complained because of the bit OR.  With the logical OR, it
shouldn't complain.

> >>
> >>  /* 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.
> 
> It's not to my taste either.

Then follow your instinct :)

> But it's in line with the existing ftu
> quirks. I'm already preparing a patch for the M-Audio C-400 mixer.
> What do you think of
> 
> * renaming the FTU specifiy functions from snd_maudio_ftu_* to snd_ftu_*
> * renaming functions common to different M-Audio devices to snd_maudio_*
> * specifically renaming snd_maudio_ftu_effect_switch_private_value to
> snd_maudio_effect_switch_priv_val (well, it's just a bit shorter...)
> 
> 
> 
> >> +     mixer = (struct usb_mixer_interface *) pval->mixer;
> >> +     if (mixer == NULL) {
> >> +             snd_printd(KERN_ERR "mixer == NULL");
> >
> > Use snd_BUG_ON() in such a case.
> 
> What's that?

It's what you need :)  Use like
	if (snd_BUG_ON(!mixer))
		return -EINVAL;


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel