[alsa-devel] [PATCH 1/1] ALSA: ASoC: Fix wrong param when call snd_soc_test_bits

[Richard Zhao]

2008/9/29 Mark Brown <broonie at sirena.org.uk>:
> On Sun, Sep 28, 2008 at 07:27:27AM +0800, Richard Zhao wrote:
>
> Please don't top post.
>
>> Without the patch, value of val is the value of the enumerated control
>> (mux), so dapm_mux_update_power didn't go wrong. But snd_soc_test_bits
>
> This appears to suggest that this is a pure coding style change but that
> doesn't tie in with any of the rest of your message or the content of
> the actual patch - what do you mean when you say that it "didn't go
> wrong"?
>
>> need the bitmask value, so I changed the original variable name "val"
>> to "mux" and add a new variable "val" which means bitmask value. So
>> the meaning of val and mux is unified with that in
>> dapm_mux_update_power.
>
> I think that what you mean to say here is that dapm_mux_update_power()
> is using val as both the index into the options for the mux and as the
> shifted value of the relevant bits in the register and these won't be
> the same unless the bits aren't shifted in the register?  If so could
> you please resubmit with a changelog entry saying so?
>
>> Though you can't find "if (!snd_soc_test_bits(widget->codec, e->reg,
>> mask, val))" in the patch, it's what I want to fix.
>
> There is no reference at all to snd_soc_test_bits() in your patch since
> you didn't change that line or anything near enough to it to include it
> in the patch and it's not immediately obvious that you've changed the
> meaning of the val argument.
>
> For future reference the main problem here is that your changelog simply
> says that you "Fix wrong param" but doesn't go into any detail on what's
> incorrect about which parameter or what's incorrect about the parameter.
>

OK, I will send out a new patch with only change log changed.

BR
Richard