[alsa-devel] [PATCH 1/1] ALSA: ASoC: Fix wrong param when call snd_soc_test_bits
Fix wrong param when call snd_soc_test_bits in function dapm_mux_update_power.
Signed-off-by: Richard Zhao linuxzsc@gmail.com
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index c016426..9ca9c08 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -693,7 +693,7 @@ static void dbg_dump_dapm(struct snd_soc_codec* codec, const char *action) /* test and update the power status of a mux widget */ static int dapm_mux_update_power(struct snd_soc_dapm_widget *widget, struct snd_kcontrol *kcontrol, int mask, - int val, struct soc_enum* e) + int mux, int val, struct soc_enum *e) { struct snd_soc_dapm_path *path; int found = 0; @@ -709,12 +709,12 @@ static int dapm_mux_update_power(struct snd_soc_dapm_widget *widget, if (path->kcontrol != kcontrol) continue;
- if (!path->name || ! e->texts[val]) + if (!path->name || !e->texts[mux]) continue;
found = 1; /* we now need to match the string in the enum to the path */ - if (!(strcmp(path->name, e->texts[val]))) + if (!(strcmp(path->name, e->texts[mux]))) path->connect = 1; /* new connection */ else path->connect = 0; /* old connection must be powered down */ @@ -1291,7 +1291,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
mutex_lock(&widget->codec->mutex); widget->value = val; - dapm_mux_update_power(widget, kcontrol, mask, mux, e); + dapm_mux_update_power(widget, kcontrol, mask, mux, val, e); if (widget->event) { if (widget->event_flags & SND_SOC_DAPM_PRE_REG) { ret = widget->event(widget,
On Sat, Sep 27, 2008 at 08:43:16PM +0800, Richard Zhao wrote:
Fix wrong param when call snd_soc_test_bits in function dapm_mux_update_power.
So... This patch doesn't actually do what the changelog entry here says: the arguments to snd_soc_test_bits() remain the same before and after your change.
if (!path->name || ! e->texts[val])
if (!path->name || !e->texts[mux]) continue;
What this is actually doing is changing the enum text examined to be indexed by mux (the value of the enumerated control) rather than val (the bitmask in the register). This looks like a good change but I want to double check with a test system.
Mark, 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 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. 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.
Thanks Richard
2008/9/28 Mark Brown broonie@opensource.wolfsonmicro.com
On Sat, Sep 27, 2008 at 08:43:16PM +0800, Richard Zhao wrote:
Fix wrong param when call snd_soc_test_bits in function dapm_mux_update_power.
So... This patch doesn't actually do what the changelog entry here says: the arguments to snd_soc_test_bits() remain the same before and after your change.
if (!path->name || ! e->texts[val])
if (!path->name || !e->texts[mux]) continue;What this is actually doing is changing the enum text examined to be indexed by mux (the value of the enumerated control) rather than val (the bitmask in the register). This looks like a good change but I want to double check with a test system.
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.
2008/9/29 Mark Brown broonie@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
participants (3)
-
Mark Brown -
Mark Brown -
Richard Zhao