ASoC: Add BUG() assertion if max98088_get_channel returns -EINVAL

PATCH In review Axel Lin 4 participants 14 messages Sep 28, 2011 – Sep 30, 2011

Subsystems (2)

Sound ASoC

lore

Share & tools

lore

Axel Lin

author

2011-09-28 14:01 UTC #0 lore raw

PATCH 1/2 ASoC: Add BUG() assertion if max98088_get_channel returns -EINVAL

The callers use the return value of max98088_get_channel as array index to access max98088->dai[] array.
Add BUG() assertion for out of bound access of max98088->dai[] array.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/max98088.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

sound/soc/codecs/max98088.c +1 -0

index ac65a2d..aaca91c 100644
@@ -1703,6 +1703,7 @@ static int max98088_get_channel(const char *name)
                return 0;
        if (strcmp(name, "EQ2 Mode") == 0)
                return 1;
+	BUG();
        return -EINVAL;
 }

Signature · 1 line

1.7.4.1

Axel Lin

author

2011-09-28 14:02 UTC #1 lore raw

PATCH 2/2 ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/max98095.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

sound/soc/codecs/max98095.c +1 -0

index 668434d..973c02d 100644
@@ -1998,6 +1998,7 @@ static int max98095_get_bq_channel(const char *name)
 		return 0;
 	if (strcmp(name, "Biquad2 Mode") == 0)
 		return 1;
+	BUG();
 	return -EINVAL;
 }

Signature · 1 line

1.7.4.1

Ryan Mallon

2011-09-28 23:19 UTC #2 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

Quoted 3 lines · Axel Lin

The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.

Same here, fix the problem in the callers.

----
Check the return value of max98095_get_bq_channel in the callers and propagate any errors up. Remove the BUG_ON(channel > 1) since max98095_get_bq_channel never returns a value larger than 1.

Signed-off-by Ryan Mallon <[email protected]>

---

sound/soc/codecs/max98095.c +5 -1

index 668434d..55eccea 100644
@@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
 	int fs, best, best_val, i;
 	int regmask, regsave;
 
-	BUG_ON(channel > 1);
+	if (channel < 0)
+		return channel;
 
 	if (!pdata || !max98095->bq_textcnt)
 		return 0;

@@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
 	int channel = max98095_get_bq_channel(kcontrol->id.name);
 	struct max98095_cdata *cdata;
 
+	if (channel < 0)
+		return channel;
+
 	cdata = &max98095->dai[channel];
 	ucontrol->value.enumerated.item[0] = cdata->bq_sel;

Dave Young

2011-09-29 01:35 UTC #3 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

Quoted 20 lines · Ryan Mallon

Quoted 3 lines · Axel Lin
The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.
Same here, fix the problem in the callers.
----
Check the return value of max98095_get_bq_channel in the callers and propagate any errors up. Remove the BUG_ON(channel > 1) since max98095_get_bq_channel never returns a value larger than 1.
Signed-off-by Ryan Mallon <[email protected]>
---
sound/soc/codecs/max98095.c +0 -0
index 668434d..55eccea 100644

sound/soc/codecs/max98095.c static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol, · line 2018

Quoted 6 lines

sound/soc/codecs/max98095.c +2 -1

@@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
        int fs, best, best_val, i;
        int regmask, regsave;
 
-       BUG_ON(channel > 1);
+       if (channel < 0)
+               return channel;

If use BUG() happens in max98095_get_bq_channel, it will not return here?

Quoted 20 lines

sound/soc/codecs/max98095.c +3 -1

        if (!pdata || !max98095->bq_textcnt)
                return 0;

@@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
        int channel = max98095_get_bq_channel(kcontrol->id.name);
        struct max98095_cdata *cdata;
 
+       if (channel < 0)
+               return channel;
+
        cdata = &max98095->dai[channel];
        ucontrol->value.enumerated.item[0] = cdata->bq_sel;
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

Regards
Dave

Ryan Mallon

2011-09-29 01:52 UTC #4 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

Quoted 24 lines · Dave Young

Quoted 23 lines · Ryan Mallon
Quoted 3 lines · Axel Lin
The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.
Same here, fix the problem in the callers.
----
Check the return value of max98095_get_bq_channel in the callers and propagate any errors up. Remove the BUG_ON(channel > 1) since max98095_get_bq_channel never returns a value larger than 1.
Signed-off-by Ryan Mallon <[email protected]>
---
sound/soc/codecs/max98095.c +0 -0
index 668434d..55eccea 100644
@@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
       int fs, best, best_val, i;
       int regmask, regsave;
 

Quoted 5 lines

sound/soc/codecs/max98095.c static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol, · line 2018
Quoted 3 lines
sound/soc/codecs/max98095.c +2 -1
-       BUG_ON(channel > 1);
+       if (channel < 0)
+               return channel;
If use BUG() happens in max98095_get_bq_channel, it will not return here?

Not quite sure what you mean?

If CONFIG_BUG was not enabled for the original version, then it would not return at the BUG_ON and would either crash or cause odd behaviour if it tried to index channel as -1.

My patch is removing the BUG_ON and replacing it with a proper check and return. It doesn't need to check > 1 since max98095_get_bq_channel never returns that.

My understanding is that device drivers, in general, should not call BUG. BUG is for unrecoverable errors which leave the kernel in some unstable state. Here we can just return an error code.

~Ryan

Quoted 24 lines

Quoted 21 lines

sound/soc/codecs/max98095.c +3 -1

       if (!pdata || !max98095->bq_textcnt)
               return 0;

@@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
       int channel = max98095_get_bq_channel(kcontrol->id.name);
       struct max98095_cdata *cdata;
 
+       if (channel < 0)
+               return channel;
+
       cdata = &max98095->dai[channel];
       ucontrol->value.enumerated.item[0] = cdata->bq_sel;
 
 
--

Dave Young

2011-09-29 01:59 UTC #5 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

Quoted 26 lines · Ryan Mallon

Quoted 24 lines · Dave Young
Quoted 23 lines · Ryan Mallon
Quoted 3 lines · Axel Lin
The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.
Same here, fix the problem in the callers.
----
Check the return value of max98095_get_bq_channel in the callers and propagate any errors up. Remove the BUG_ON(channel > 1) since max98095_get_bq_channel never returns a value larger than 1.
Signed-off-by Ryan Mallon <[email protected]>
---
sound/soc/codecs/max98095.c +0 -0
index 668434d..55eccea 100644
@@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
        int fs, best, best_val, i;
        int regmask, regsave;
 

Quoted 8 lines

Quoted 5 lines
sound/soc/codecs/max98095.c static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol, · line 2018
Quoted 3 lines
sound/soc/codecs/max98095.c +2 -1
-       BUG_ON(channel > 1);
+       if (channel < 0)
+               return channel;
If use BUG() happens in max98095_get_bq_channel, it will not return here?
Not quite sure what you mean?

I means if Axel Lin's patch applied, and CONFIG_BUG is on, it will panic firstly the if condition will be never entered.

Quoted 5 lines

If CONFIG_BUG was not enabled for the original version, then it would not return at the BUG_ON and would either crash or cause odd behaviour if it tried to index channel as -1.

Quoted 7 lines

My patch is removing the BUG_ON and replacing it with a proper check and return. It doesn't need to check > 1 since max98095_get_bq_channel never returns that.
My understanding is that device drivers, in general, should not call BUG. BUG is for unrecoverable errors which leave the kernel in some unstable state. Here we can just return an error code.

Agree

Quoted 30 lines

~Ryan

Quoted 24 lines

Quoted 21 lines

sound/soc/codecs/max98095.c +3 -1

        if (!pdata || !max98095->bq_textcnt)
                return 0;

@@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
        int channel = max98095_get_bq_channel(kcontrol->id.name);
        struct max98095_cdata *cdata;
 
+       if (channel < 0)
+               return channel;
+
        cdata = &max98095->dai[channel];
        ucontrol->value.enumerated.item[0] = cdata->bq_sel;
 
 
--

Signature · 2 lines

Regards
Dave

Ryan Mallon

2011-09-29 02:01 UTC #6 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

Quoted 34 lines · Dave Young

Quoted 33 lines · Ryan Mallon
Quoted 29 lines · Dave Young
Quoted 20 lines · Ryan Mallon
Quoted 3 lines · Axel Lin
The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.
Same here, fix the problem in the callers.
----
Check the return value of max98095_get_bq_channel in the callers and propagate any errors up. Remove the BUG_ON(channel > 1) since max98095_get_bq_channel never returns a value larger than 1.
Signed-off-by Ryan Mallon <[email protected]>
---
sound/soc/codecs/max98095.c +0 -0
index 668434d..55eccea 100644
sound/soc/codecs/max98095.c static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol, · line 2018
Quoted 6 lines
sound/soc/codecs/max98095.c +2 -1
@@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
       int fs, best, best_val, i;
       int regmask, regsave;
 
-       BUG_ON(channel > 1);
+       if (channel < 0)
+               return channel;
If use BUG() happens in max98095_get_bq_channel, it will not return here?

Quoted 4 lines

Quoted 1 line
Not quite sure what you mean?
I means if Axel Lin's patch applied, and CONFIG_BUG is on, it will panic firstly the if condition will be never entered.

My patch is a replacement for Axel's patch, not on top of it. For Axel's patch it would panic if channel was less than zero if CONFIG_BUG was enabled, but would still have undefined behaviour if CONFIG_BUG was not enabled.

~Ryan

Quoted 48 lines

Quoted 5 lines
If CONFIG_BUG was not enabled for the original version, then it would not return at the BUG_ON and would either crash or cause odd behaviour if it tried to index channel as -1.
Quoted 7 lines
My patch is removing the BUG_ON and replacing it with a proper check and return. It doesn't need to check > 1 since max98095_get_bq_channel never returns that.
My understanding is that device drivers, in general, should not call BUG. BUG is for unrecoverable errors which leave the kernel in some unstable state. Here we can just return an error code.
Agree
Quoted 30 lines
~Ryan
Quoted 24 lines
Quoted 21 lines
sound/soc/codecs/max98095.c +3 -1
       if (!pdata || !max98095->bq_textcnt)
               return 0;
@@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
       int channel = max98095_get_bq_channel(kcontrol->id.name);
       struct max98095_cdata *cdata;
 
+       if (channel < 0)
+               return channel;
+
       cdata = &max98095->dai[channel];
       ucontrol->value.enumerated.item[0] = cdata->bq_sel;
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Dave Young

2011-09-29 02:06 UTC #7 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

Quoted 38 lines · Ryan Mallon

Quoted 36 lines · Dave Young
Quoted 34 lines · Ryan Mallon
Quoted 24 lines · Dave Young
Quoted 23 lines · Ryan Mallon
Quoted 3 lines · Axel Lin
The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.
Same here, fix the problem in the callers.
----
Check the return value of max98095_get_bq_channel in the callers and propagate any errors up. Remove the BUG_ON(channel > 1) since max98095_get_bq_channel never returns a value larger than 1.
Signed-off-by Ryan Mallon <[email protected]>
---
sound/soc/codecs/max98095.c +0 -0
index 668434d..55eccea 100644
@@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
        int fs, best, best_val, i;
        int regmask, regsave;
 
Quoted 5 lines
sound/soc/codecs/max98095.c static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol, · line 2018
Quoted 3 lines
sound/soc/codecs/max98095.c +2 -1
-       BUG_ON(channel > 1);
+       if (channel < 0)
+               return channel;
If use BUG() happens in max98095_get_bq_channel, it will not return here?
Not quite sure what you mean?

Quoted 7 lines

Quoted 2 lines
I means if Axel Lin's patch applied, and CONFIG_BUG is on, it will panic firstly the if condition will be never entered.
My patch is a replacement for Axel's patch, not on top of it. For Axel's patch it would panic if channel was less than zero if CONFIG_BUG was enabled, but would still have undefined behaviour if CONFIG_BUG was not enabled.

So that's good, thanks

Quoted 54 lines

~Ryan
Quoted 48 lines
Quoted 5 lines
If CONFIG_BUG was not enabled for the original version, then it would not return at the BUG_ON and would either crash or cause odd behaviour if it tried to index channel as -1.
Quoted 7 lines
My patch is removing the BUG_ON and replacing it with a proper check and return. It doesn't need to check > 1 since max98095_get_bq_channel never returns that.
My understanding is that device drivers, in general, should not call BUG. BUG is for unrecoverable errors which leave the kernel in some unstable state. Here we can just return an error code.
Agree
Quoted 30 lines
~Ryan
Quoted 24 lines
Quoted 21 lines
sound/soc/codecs/max98095.c +3 -1
        if (!pdata || !max98095->bq_textcnt)
                return 0;
@@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
        int channel = max98095_get_bq_channel(kcontrol->id.name);
        struct max98095_cdata *cdata;
 
+       if (channel < 0)
+               return channel;
+
        cdata = &max98095->dai[channel];
        ucontrol->value.enumerated.item[0] = cdata->bq_sel;
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

Regards
Dave

Dave Young

2011-09-29 01:33 UTC #8 lore raw

Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel returns -EINVAL

In reply to Axel Lin

Quoted 14 lines · Axel Lin

The callers use the return value of max98095_get_bq_channel as array index to access max98095->dai[] array.
Add BUG() assertion for out of bound access of max98095->dai[] array.
Signed-off-by Axel Lin <[email protected]>
---
sound/soc/codecs/max98095.c | 1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
sound/soc/codecs/max98095.c +0 -0
index 668434d..973c02d 100644

sound/soc/codecs/max98095.c static int max98095_get_bq_channel(const char *name) · line 2002

Quoted 5 lines

sound/soc/codecs/max98095.c +1 -0

@@ -1998,6 +1998,7 @@ static int max98095_get_bq_channel(const char *name)
                return 0;
        if (strcmp(name, "Biquad2 Mode") == 0)
                return 1;
+       BUG();
        return -EINVAL;

below better?
BUG_ON(strcmp(name, "Biquad2 Mode"));
return 1;

Or BUG_ON(channel < 0) in caller

Quoted 13 lines

sound/soc/codecs/max98095.c +0 -1
  }
 
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

Regards
Dave

Ryan Mallon

2011-09-28 23:15 UTC #9 lore raw

In reply to Axel Lin

Quoted 3 lines · Axel Lin

The callers use the return value of max98088_get_channel as array index to access max98088->dai[] array.
Add BUG() assertion for out of bound access of max98088->dai[] array.

BUG() is pretty heavy handed for a driver. Why not fix the problem properly in the callers?

----
Check the return value of max98088_get_channel in the callers and propagate any errors up.

Signed-off-by Ryan Mallon <[email protected]>

---

sound/soc/codecs/max98088.c +6 -0

index ac65a2d..e07700e 100644
@@ -1811,6 +1811,9 @@ static int max98088_put_eq_enum(struct snd_kcontrol *kcontrol,
        struct max98088_cdata *cdata;
        int sel = ucontrol->value.integer.value[0];
 
+       if (channel < 0)
+	       return channel;
+
        cdata = &max98088->dai[channel];
 
        if (sel >= pdata->eq_cfgcnt)

@@ -1838,6 +1841,9 @@ static int max98088_get_eq_enum(struct snd_kcontrol *kcontrol,
        int channel = max98088_get_channel(kcontrol->id.name);
        struct max98088_cdata *cdata;
 
+       if (channel < 0)
+	       return channel;
+
        cdata = &max98088->dai[channel];
        ucontrol->value.enumerated.item[0] = cdata->eq_sel;
        return 0;

Mark Brown

2011-09-29 10:34 UTC #10 lore raw

Quoted 7 lines · Ryan Mallon

Quoted 3 lines · Axel Lin
The callers use the return value of max98088_get_channel as array index to access max98088->dai[] array.
Add BUG() assertion for out of bound access of max98088->dai[] array.
BUG() is pretty heavy handed for a driver. Why not fix the problem properly in the callers?

There's nothing constructive that any of the callers can do with an error code - it's a clear bug in something (probably the driver) if we get called for a bad control. Simply returning an error code isn't terribly helpful, it's very obscure what's gone wrong and why. We at least need a log message.

Ryan Mallon

2011-09-29 11:25 UTC #11 lore raw

Quoted 5 lines · Mark Brown

Quoted 4 lines · Ryan Mallon
Quoted 3 lines · Axel Lin
The callers use the return value of max98088_get_channel as array index to access max98088->dai[] array.
Add BUG() assertion for out of bound access of max98088->dai[] array.

Quoted 7 lines

Quoted 2 lines
BUG() is pretty heavy handed for a driver. Why not fix the problem properly in the callers?
There's nothing constructive that any of the callers can do with an error code - it's a clear bug in something (probably the driver) if we get called for a bad control. Simply returning an error code isn't terribly helpful, it's very obscure what's gone wrong and why. We at least need a log message.

Yeah, it can basically only happen if there is a mismatch between the kcontrol definition and the get_channel function in the driver. Would you be happy with adding a:

  dev_err(codec->dev, "Bad kcontrol channel name\n");

and then returning the error? It doesn't seem worth panicking the whole driver/system for a bug like this.

~Ryan

Ryan Mallon

2011-09-29 23:13 UTC #12 lore raw

In reply to Mark Brown

Quoted 5 lines · Mark Brown

Quoted 4 lines · Ryan Mallon
Quoted 3 lines · Axel Lin
The callers use the return value of max98088_get_channel as array index to access max98088->dai[] array.
Add BUG() assertion for out of bound access of max98088->dai[] array.

Quoted 7 lines

Quoted 2 lines
BUG() is pretty heavy handed for a driver. Why not fix the problem properly in the callers?
There's nothing constructive that any of the callers can do with an error code - it's a clear bug in something (probably the driver) if we get called for a bad control. Simply returning an error code isn't terribly helpful, it's very obscure what's gone wrong and why. We at least need a log message.

How about something like this (untested) patch?
---
Make the max98088 codec controls[] array a static global and iterate over it in max98088_get_channel rather than duplicating the hardcoded strings. Add a warning if the channel name is not found and check for the error value in the callers.

Signed-off-by Ryan Mallon <[email protected]>

---

sound/soc/codecs/max98088.c +29 ~~-19~~

index ac65a2d..bc2b922 100644
@@ -1697,13 +1697,28 @@ static struct snd_soc_dai_driver max98088_dai[] = {
 }
 };
 
-static int max98088_get_channel(const char *name)
+static struct snd_kcontrol_new controls[] = {
+	SOC_ENUM_EXT("EQ1 Mode",
+		     max98088->eq_enum,
+		     max98088_get_eq_enum,
+		     max98088_put_eq_enum),
+	SOC_ENUM_EXT("EQ2 Mode",
+		     max98088->eq_enum,
+		     max98088_get_eq_enum,
+		     max98088_put_eq_enum),
+};
+
+static int max98088_get_channel(struct snd_soc_codec *codec, const char *name)
 {
-       if (strcmp(name, "EQ1 Mode") == 0)
-               return 0;
-       if (strcmp(name, "EQ2 Mode") == 0)
-               return 1;
-       return -EINVAL;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(controls); i++)
+		if (strcmp(name, controls[i].name) == 0)
+			return i;
+
+	/* Shouldn't happen */
+	dev_err(codec->dev, "Bad channel name '%s'\n", name);
+	return -EINVAL;
 }
 
 static void max98088_setup_eq1(struct snd_soc_codec *codec)

@@ -1807,10 +1822,13 @@ static int max98088_put_eq_enum(struct snd_kcontrol *kcontrol,
        struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
        struct max98088_priv *max98088 = snd_soc_codec_get_drvdata(codec);
        struct max98088_pdata *pdata = max98088->pdata;
-       int channel = max98088_get_channel(kcontrol->id.name);
+       int channel = max98088_get_channel(codec, kcontrol->id.name);
        struct max98088_cdata *cdata;
        int sel = ucontrol->value.integer.value[0];
 
+       if (channel < 0)
+	       return channel;
+
        cdata = &max98088->dai[channel];
 
        if (sel >= pdata->eq_cfgcnt)

@@ -1835,9 +1853,12 @@ static int max98088_get_eq_enum(struct snd_kcontrol *kcontrol,
 {
        struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
        struct max98088_priv *max98088 = snd_soc_codec_get_drvdata(codec);
-       int channel = max98088_get_channel(kcontrol->id.name);
+       int channel = max98088_get_channel(codec, kcontrol->id.name);
        struct max98088_cdata *cdata;
 
+       if (channel < 0)
+	       return channel;
+
        cdata = &max98088->dai[channel];
        ucontrol->value.enumerated.item[0] = cdata->eq_sel;
        return 0;

@@ -1853,17 +1874,6 @@ static void max98088_handle_eq_pdata(struct snd_soc_codec *codec)
        const char **t;
        int ret;
 
-       struct snd_kcontrol_new controls[] = {
-               SOC_ENUM_EXT("EQ1 Mode",
-                       max98088->eq_enum,
-                       max98088_get_eq_enum,
-                       max98088_put_eq_enum),
-               SOC_ENUM_EXT("EQ2 Mode",
-                       max98088->eq_enum,
-                       max98088_get_eq_enum,
-                       max98088_put_eq_enum),
-       };
-
        cfg = pdata->eq_cfg;
        cfgcnt = pdata->eq_cfgcnt;

Mark Brown

2011-09-30 12:56 UTC #13 lore raw

Quoted 1 line · Ryan Mallon

How about something like this (untested) patch?

Looks plausible.