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

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

[Richard Zhao]

From: Richard Zhao <richard@richard-laptop.(none)>

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,
-- 
1.5.4.3

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

[Richard Zhao]

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,
-- 
1.5.4.3

Re: [PATCH 1/1] ALSA: ASoC: Fix wrong param when call snd_soc_test_bits

[Mark Brown]

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.

Re: [PATCH 1/1] ALSA: ASoC: Fix wrong param when call snd_soc_test_bits

[Richard Zhao]

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.

Re: [PATCH 1/1] ALSA: ASoC: Fix wrong param when call snd_soc_test_bits

[Mark Brown]

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.

Re: [PATCH 1/1] ALSA: ASoC: Fix wrong param when call snd_soc_test_bits

[Richard Zhao]

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