On 7/23/08, Mark Brown broonie@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;