[alsa-devel] [PATCH] Convert bitfields in ASOC into full int width

[Jon Smirl]

On 7/23/08, Mark Brown <broonie at opensource.wolfsonmicro.com> wrote:

I found the brace problem.

>  > @@ -104,10 +104,12 @@ static void dapm_set_path_status(struct snd_soc_dapm_widget *w,
>  >       case snd_soc_dapm_switch:
>  >       case snd_soc_dapm_mixer: {
>  >               int val;
>  > -             int reg = w->kcontrols[i].private_value & 0xff;
>  > -             int shift = (w->kcontrols[i].private_value >> 8) & 0x0f;
>  > -             int mask = (w->kcontrols[i].private_value >> 16) & 0xff;
>  > -             int invert = (w->kcontrols[i].private_value >> 24) & 0x01;
>  > +             struct mixer_control *mc = (struct mixer_control *)w->kcontrols[i].private_value;
>  > +             uint reg = mc->reg;
>  > +             uint shift = mc->shift;
>  > +             int max = mc->max;
>  > +             uint mask = (1 << fls(max)) - 1;
>  > +             uint invert = mc->invert;
>
>
> What the original code is doing here is checking to see if the control
>  is on at all.
>

Mask is usually the same as mask, but hasn't this been written to
allow a max smaller than the max? If so these two areas look wrong.

	if (widget->num_kcontrols && k) {
		struct soc_mixer_control *mc = (struct soc_mixer_control *)k->private_value;
		uint reg = mc->reg;
		uint shift = mc->shift;
		int max = mc->max;
		uint mask = (1 << fls(max)) - 1;
		uint invert = mc->invert;

		if (power) {
			int i;
			/* power up has happended, increase volume to last level */
			if (invert) {
// shouldn't this be
//				for (i = max; i > widget->saved_value; i--)
				for (i = mask; i > widget->saved_value; i--)
// and this mask is right
					snd_soc_update_bits(widget->codec, reg, mask, i);
			} else {
				for (i = 0; i < widget->saved_value; i++)
					snd_soc_update_bits(widget->codec, reg, mask, i);
			}

And this one should have used mask instead of max:

	case snd_soc_dapm_mixer: {
		int val;
		struct soc_mixer_control *mc = (struct soc_mixer_control
*)w->kcontrols[i].private_value;
		uint reg = mc->reg;
		uint shift = mc->shift;
		int max = mc->max;
		uint mask = (1 << fls(max)) - 1;
		uint invert = mc->invert;

		val = snd_soc_read(w->codec, reg);
		val = (val >> shift) & mask;

		if ((invert && !val) || (!invert && val))
			p->connect = 1;
		else
			p->connect = 0;


-- 
Jon Smirl
jonsmirl at gmail.com