[alsa-devel] [PATCH 2/4] snd-usb-6fire: add individual volume control for analog channels

[Takashi Iwai]

At Thu, 12 Jan 2012 00:15:56 +0100,
Torsten Schenk wrote:
> 
>  /*
> - * calculated with $value\[i\] = 128 \cdot sqrt[3]{\frac{i}{128}}$
> - * this is done because the linear values cause rapid degredation
> - * of volume in the uppermost region.
> - */
> -static const u8 log_volume_table[128] = {
> -	0x00, 0x19, 0x20, 0x24, 0x28, 0x2b, 0x2e, 0x30, 0x32, 0x34,
> -	0x36, 0x38, 0x3a, 0x3b, 0x3d, 0x3e, 0x40, 0x41, 0x42, 0x43,
> -	0x44, 0x46, 0x47, 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e,
> -	0x4e, 0x4f, 0x50, 0x51, 0x52, 0x53, 0x53, 0x54, 0x55, 0x56,
> -	0x56, 0x57, 0x58, 0x58, 0x59, 0x5a, 0x5b, 0x5b, 0x5c, 0x5c,
> -	0x5d, 0x5e, 0x5e, 0x5f, 0x60, 0x60, 0x61, 0x61, 0x62, 0x62,
> -	0x63, 0x63, 0x64, 0x65, 0x65, 0x66, 0x66, 0x67, 0x67, 0x68,
> -	0x68, 0x69, 0x69, 0x6a, 0x6a, 0x6b, 0x6b, 0x6c, 0x6c, 0x6c,
> -	0x6d, 0x6d, 0x6e, 0x6e, 0x6f, 0x6f, 0x70, 0x70, 0x70, 0x71,
> -	0x71, 0x72, 0x72, 0x73, 0x73, 0x73, 0x74, 0x74, 0x75, 0x75,
> -	0x75, 0x76, 0x76, 0x77, 0x77, 0x77, 0x78, 0x78, 0x78, 0x79,
> -	0x79, 0x7a, 0x7a, 0x7a, 0x7b, 0x7b, 0x7b, 0x7c, 0x7c, 0x7c,
> -	0x7d, 0x7d, 0x7d, 0x7e, 0x7e, 0x7e, 0x7f, 0x7f };

So, with the new implementation, you don't need the linear -> dB
conversion?

> -static int usb6fire_control_master_vol_put(struct snd_kcontrol *kcontrol,
> +static int usb6fire_control_output_vol_put(struct snd_kcontrol *kcontrol,
>  		struct snd_ctl_elem_value *ucontrol)
>  {
>  	struct control_runtime *rt = snd_kcontrol_chip(kcontrol);
> +	unsigned long chs = kcontrol->private_value;
> +	unsigned long ch0 = 2 * chs;
> +	unsigned long ch1 = 2 * chs + 1;
>  	int changed = 0;
> -	if (rt->master_vol != ucontrol->value.integer.value[0]) {
> -		rt->master_vol = ucontrol->value.integer.value[0];
> -		usb6fire_control_master_vol_update(rt);
> +
> +	if (chs < 0 || chs > 2) {
> +		snd_printk(KERN_ERR PREFIX "Invalid channel in volume control.");
> +		return 0;

Better to return -EINVAL or such.


> -static int usb6fire_control_master_vol_get(struct snd_kcontrol *kcontrol,
> +static int usb6fire_control_output_vol_get(struct snd_kcontrol *kcontrol,
>  		struct snd_ctl_elem_value *ucontrol)
>  {
>  	struct control_runtime *rt = snd_kcontrol_chip(kcontrol);
> -	ucontrol->value.integer.value[0] = rt->master_vol;
> +	unsigned long chs = kcontrol->private_value;
> +	unsigned long ch0 = 2 * chs;
> +	unsigned long ch1 = 2 * chs + 1;
> +
> +	if (chs < 0 || chs > 2) {
> +		snd_printk(KERN_ERR PREFIX "Invalid channel in volume control.");
> +		return 0;

Ditto.

> -static struct __devinitdata snd_kcontrol_new elements[] = {
> +static struct __devinitdata snd_kcontrol_new vol_elements[] = {
> +	{
> +		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +		.name = "Output 1/2 Playback Volume",
> +		.index = 0,
> +		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> +		.info = usb6fire_control_output_vol_info,
> +		.get = usb6fire_control_output_vol_get,
> +		.put = usb6fire_control_output_vol_put,
> +		.private_value = 0
> +	},
>  	{
>  		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> -		.name = "Master Playback Volume",
> +		.name = "Output 3/4 Playback Volume",
>  		.index = 0,

Keep the same name string like "Output Playback Volume" with different
index numbers.

>  		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> -		.info = usb6fire_control_master_vol_info,
> -		.get = usb6fire_control_master_vol_get,
> -		.put = usb6fire_control_master_vol_put
> +		.info = usb6fire_control_output_vol_info,
> +		.get = usb6fire_control_output_vol_get,
> +		.put = usb6fire_control_output_vol_put,
> +		.private_value = 1
>  	},
>  	{
>  		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +		.name = "Output 5/6 Playback Volume",
> +		.index = 0,
> +		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> +		.info = usb6fire_control_output_vol_info,
> +		.get = usb6fire_control_output_vol_get,
> +		.put = usb6fire_control_output_vol_put,
> +		.private_value = 2
> +	}
> +};

Add the terminator in this patch.  I know you fixed in the last patch
but, In general, try not to introduce a bug in each commit.  Otherwise
it'll make bisection difficult.


thanks,

Takashi