All of lore.kernel.org
 help / color / mirror / Atom feed
* stac92xx_parse_auto_config() can return 0 on error?
@ 2007-02-05 21:40 Jason Baron
  2007-02-05 22:29 ` Tobin Davis
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Baron @ 2007-02-05 21:40 UTC (permalink / raw)
  To: alsa-devel; +Cc: John Linville


hi,

I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
because the 'imux' parameter pointer was NULL. I traced this back to 
stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
set, without having initialized the 'input_mux'.

The oops does not happen on the upstream kernel on this hardware b/c 
'line_outs' was set (additional code has been added since RHEL4 to set 
it). However, it still appears to me, that if 'line_outs' is not set, we 
should return an error code, and not 0.

thanks,

-Jason

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: stac92xx_parse_auto_config() can return 0 on error?
  2007-02-05 21:40 stac92xx_parse_auto_config() can return 0 on error? Jason Baron
@ 2007-02-05 22:29 ` Tobin Davis
  2007-02-06 11:47   ` Takashi Iwai
  0 siblings, 1 reply; 7+ messages in thread
From: Tobin Davis @ 2007-02-05 22:29 UTC (permalink / raw)
  To: Jason Baron; +Cc: alsa-devel, John Linville


[-- Attachment #1.1: Type: text/plain, Size: 987 bytes --]

On Mon, 2007-02-05 at 16:40 -0500, Jason Baron wrote:

> hi,
> 
> I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
> because the 'imux' parameter pointer was NULL. I traced this back to 
> stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
> set, without having initialized the 'input_mux'.
> 
> The oops does not happen on the upstream kernel on this hardware b/c 
> 'line_outs' was set (additional code has been added since RHEL4 to set 
> it). However, it still appears to me, that if 'line_outs' is not set, we 
> should return an error code, and not 0.
> 
> thanks,
> 
> -Jason

Try this patch to see if it works with your alsa version.  It's a simple
patch that adds the error condition, and should be able to be applied to
older alsa versions.

Summary:  add error for undetected line_outs.

This adds an error condition to stac92xx_parse_auto_config if line_outs
is less than zero.


Signed off by Tobin Davis <tdavis@dsl-only.net>

[-- Attachment #1.2: Type: text/html, Size: 1681 bytes --]

[-- Attachment #2: stax92xx-autocfg-fix.patch --]
[-- Type: text/x-patch, Size: 558 bytes --]

diff -r c783393557a8 pci/hda/patch_sigmatel.c
--- a/pci/hda/patch_sigmatel.c	Wed Jan 24 16:13:35 2007 +0100
+++ b/pci/hda/patch_sigmatel.c	Mon Feb 05 14:24:57 2007 -0800
@@ -1375,8 +1375,8 @@ static int stac92xx_parse_auto_config(st
 						&spec->autocfg,
 						spec->dmic_nids)) < 0)
 		return err;
-	if (! spec->autocfg.line_outs)
-		return 0; /* can't find valid pin config */
+	if ((err = spec->autocfg.line_outs) < 0)
+		return err; /* can't find valid pin config */
 
 	if ((err = stac92xx_add_dyn_out_pins(codec, &spec->autocfg)) < 0)
 		return err;

[-- Attachment #3: Type: text/plain, Size: 374 bytes --]

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: stac92xx_parse_auto_config() can return 0 on error?
  2007-02-05 22:29 ` Tobin Davis
@ 2007-02-06 11:47   ` Takashi Iwai
  2007-02-06 16:53     ` Jason Baron
  2007-02-08 15:30     ` Takashi Iwai
  0 siblings, 2 replies; 7+ messages in thread
From: Takashi Iwai @ 2007-02-06 11:47 UTC (permalink / raw)
  To: Tobin Davis; +Cc: Jason Baron, John Linville, alsa-devel

At Mon, 05 Feb 2007 14:29:53 -0800,
Tobin Davis wrote:
> 
> On Mon, 2007-02-05 at 16:40 -0500, Jason Baron wrote:
> 
>     hi,
>     
>     I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
>     because the 'imux' parameter pointer was NULL. I traced this back to 
>     stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
>     set, without having initialized the 'input_mux'.
>     
>     The oops does not happen on the upstream kernel on this hardware b/c 
>     'line_outs' was set (additional code has been added since RHEL4 to set 
>     it). However, it still appears to me, that if 'line_outs' is not set, we 
>     should return an error code, and not 0.
>     
>     thanks,
>     
>     -Jason
> 
> Try this patch to see if it works with your alsa version.  It's a simple patch that
> adds the error condition, and should be able to be applied to older alsa versions.
> 
> Summary:  add error for undetected line_outs.
> 
> This adds an error condition to stac92xx_parse_auto_config if line_outs is less than
> zero.
> 

line_outs won't be negative, so this situation can't happen.
The problem is that stac92xx_parse_auto_config() returns "0" (which
means successful) even if line_outs = 0 and thus nothing is
configured.  Simply changing it to return -EINVAL would work. 

However, this is no best solution.  In the case of patch_realtek.c or
others, the codec probe routine behaves slightly differently. 
*_parse_auto_config() may return 0 if no ane configuration is found.
Then it falls back to model=basic.  Returning a negative means a fatal
error to stop.  Meanwhile, in the case of sigmatel, there is no
fallback, so it always stop.

The patch below adds the fallback to model=ref in case that no proper
auto-config is available.  Please give it a try.


Takashi


diff -r a42a457ae20c pci/hda/patch_sigmatel.c
--- a/pci/hda/patch_sigmatel.c	Mon Feb 05 14:56:20 2007 +0100
+++ b/pci/hda/patch_sigmatel.c	Tue Feb 06 12:45:36 2007 +0100
@@ -1800,6 +1800,7 @@ static int patch_stac925x(struct hda_cod
 	spec->board_config = snd_hda_check_board_config(codec, STAC_925x_MODELS,
 							stac925x_models,
 							stac925x_cfg_tbl);
+ again:
 	if (spec->board_config < 0) {
 		snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC925x, using BIOS defaults\n");
 		err = stac92xx_save_bios_config_regs(codec);
@@ -1825,6 +1826,15 @@ static int patch_stac925x(struct hda_cod
 	spec->mixer = stac925x_mixer;
 
 	err = stac92xx_parse_auto_config(codec, 0x8, 0x7);
+	if (!err) {
+		if (spec->board_config < 0) {
+			printk(KERN_WARNING "hda_codec: No auto-config is "
+			       "available, default to model=ref\n");
+			spec->board_config = STAC_925x_REF;
+			goto again;
+		}
+		err = -EINVAL;
+	}
 	if (err < 0) {
 		stac92xx_free(codec);
 		return err;
@@ -1850,6 +1860,7 @@ static int patch_stac922x(struct hda_cod
 	spec->board_config = snd_hda_check_board_config(codec, STAC_922X_MODELS,
 							stac922x_models,
 							stac922x_cfg_tbl);
+ again:
 	if (spec->board_config < 0) {
 		snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC922x, "
 			"using BIOS defaults\n");
@@ -1875,6 +1886,15 @@ static int patch_stac922x(struct hda_cod
 	spec->multiout.dac_nids = spec->dac_nids;
 	
 	err = stac92xx_parse_auto_config(codec, 0x08, 0x09);
+	if (!err) {
+		if (spec->board_config < 0) {
+			printk(KERN_WARNING "hda_codec: No auto-config is "
+			       "available, default to model=ref\n");
+			spec->board_config = STAC_D945_REF;
+			goto again;
+		}
+		err = -EINVAL;
+	}
 	if (err < 0) {
 		stac92xx_free(codec);
 		return err;
@@ -1903,6 +1923,7 @@ static int patch_stac927x(struct hda_cod
 	spec->board_config = snd_hda_check_board_config(codec, STAC_927X_MODELS,
 							stac927x_models,
 							stac927x_cfg_tbl);
+ again:
 	if (spec->board_config < 0) {
                 snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC927x, using BIOS defaults\n");
 		err = stac92xx_save_bios_config_regs(codec);
@@ -1945,6 +1966,15 @@ static int patch_stac927x(struct hda_cod
 	spec->multiout.dac_nids = spec->dac_nids;
 
 	err = stac92xx_parse_auto_config(codec, 0x1e, 0x20);
+	if (!err) {
+		if (spec->board_config < 0) {
+			printk(KERN_WARNING "hda_codec: No auto-config is "
+			       "available, default to model=ref\n");
+			spec->board_config = STAC_D965_REF;
+			goto again;
+		}
+		err = -EINVAL;
+	}
 	if (err < 0) {
 		stac92xx_free(codec);
 		return err;
@@ -1970,6 +2000,7 @@ static int patch_stac9205(struct hda_cod
 	spec->board_config = snd_hda_check_board_config(codec, STAC_9205_MODELS,
 							stac9205_models,
 							stac9205_cfg_tbl);
+ again:
 	if (spec->board_config < 0) {
 		snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC9205, using BIOS defaults\n");
 		err = stac92xx_save_bios_config_regs(codec);
@@ -2008,6 +2039,15 @@ static int patch_stac9205(struct hda_cod
 			    AC_VERB_SET_GPIO_MASK, 0x00000001);
 
 	err = stac92xx_parse_auto_config(codec, 0x1f, 0x20);
+	if (!err) {
+		if (spec->board_config < 0) {
+			printk(KERN_WARNING "hda_codec: No auto-config is "
+			       "available, default to model=ref\n");
+			spec->board_config = STAC_9205_REF;
+			goto again;
+		}
+		err = -EINVAL;
+	}
 	if (err < 0) {
 		stac92xx_free(codec);
 		return err;

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: stac92xx_parse_auto_config() can return 0 on error?
  2007-02-06 11:47   ` Takashi Iwai
@ 2007-02-06 16:53     ` Jason Baron
  2007-02-06 17:00       ` Takashi Iwai
  2007-02-08 15:30     ` Takashi Iwai
  1 sibling, 1 reply; 7+ messages in thread
From: Jason Baron @ 2007-02-06 16:53 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: John Linville, alsa-devel


On Tue, 6 Feb 2007, Takashi Iwai wrote:

> At Mon, 05 Feb 2007 14:29:53 -0800,
> Tobin Davis wrote:
> > 
> > On Mon, 2007-02-05 at 16:40 -0500, Jason Baron wrote:
> > 
> >     hi,
> >     
> >     I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
> >     because the 'imux' parameter pointer was NULL. I traced this back to 
> >     stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
> >     set, without having initialized the 'input_mux'.
> >     
> >     The oops does not happen on the upstream kernel on this hardware b/c 
> >     'line_outs' was set (additional code has been added since RHEL4 to set 
> >     it). However, it still appears to me, that if 'line_outs' is not set, we 
> >     should return an error code, and not 0.
> >     
> >     thanks,
> >     
> >     -Jason
> > 
> > Try this patch to see if it works with your alsa version.  It's a simple patch that
> > adds the error condition, and should be able to be applied to older alsa versions.
> > 
> > Summary:  add error for undetected line_outs.
> > 
> > This adds an error condition to stac92xx_parse_auto_config if line_outs is less than
> > zero.
> > 
> 
> line_outs won't be negative, so this situation can't happen.
> The problem is that stac92xx_parse_auto_config() returns "0" (which
> means successful) even if line_outs = 0 and thus nothing is
> configured.  Simply changing it to return -EINVAL would work. 
> 
> However, this is no best solution.  In the case of patch_realtek.c or
> others, the codec probe routine behaves slightly differently. 
> *_parse_auto_config() may return 0 if no ane configuration is found.
> Then it falls back to model=basic.  Returning a negative means a fatal
> error to stop.  Meanwhile, in the case of sigmatel, there is no
> fallback, so it always stop.
> 
> The patch below adds the fallback to model=ref in case that no proper
> auto-config is available.  Please give it a try.
> 
> 
> Takashi
> 
> 

ok. thanks. in our backport, we don't have 'STAC_925x_REF' 
defined....should we be using 'STAC_REF' instead? Also, the change that 
introduced this problem into our 'beta' tree was:

@@ -855,14 +1137,14 @@ static int stac92xx_parse_auto_config(st

        if ((err = snd_hda_parse_pin_def_config(codec, &spec->autocfg, 
NULL)) < 0)
                return err;
-       if (! spec->autocfg.line_outs && ! spec->autocfg.hp_pin)
+       if (! spec->autocfg.line_outs)
                return 0; /* can't find valid pin config */


If i revert this single change, i avoid the oops as well....

thanks.

-Jason

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: stac92xx_parse_auto_config() can return 0 on error?
  2007-02-06 16:53     ` Jason Baron
@ 2007-02-06 17:00       ` Takashi Iwai
  0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2007-02-06 17:00 UTC (permalink / raw)
  To: Jason Baron; +Cc: John Linville, alsa-devel

At Tue, 6 Feb 2007 11:53:21 -0500 (EST),
Jason Baron wrote:
> 
> 
> On Tue, 6 Feb 2007, Takashi Iwai wrote:
> 
> > At Mon, 05 Feb 2007 14:29:53 -0800,
> > Tobin Davis wrote:
> > > 
> > > On Mon, 2007-02-05 at 16:40 -0500, Jason Baron wrote:
> > > 
> > >     hi,
> > >     
> > >     I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
> > >     because the 'imux' parameter pointer was NULL. I traced this back to 
> > >     stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
> > >     set, without having initialized the 'input_mux'.
> > >     
> > >     The oops does not happen on the upstream kernel on this hardware b/c 
> > >     'line_outs' was set (additional code has been added since RHEL4 to set 
> > >     it). However, it still appears to me, that if 'line_outs' is not set, we 
> > >     should return an error code, and not 0.
> > >     
> > >     thanks,
> > >     
> > >     -Jason
> > > 
> > > Try this patch to see if it works with your alsa version.  It's a simple patch that
> > > adds the error condition, and should be able to be applied to older alsa versions.
> > > 
> > > Summary:  add error for undetected line_outs.
> > > 
> > > This adds an error condition to stac92xx_parse_auto_config if line_outs is less than
> > > zero.
> > > 
> > 
> > line_outs won't be negative, so this situation can't happen.
> > The problem is that stac92xx_parse_auto_config() returns "0" (which
> > means successful) even if line_outs = 0 and thus nothing is
> > configured.  Simply changing it to return -EINVAL would work. 
> > 
> > However, this is no best solution.  In the case of patch_realtek.c or
> > others, the codec probe routine behaves slightly differently. 
> > *_parse_auto_config() may return 0 if no ane configuration is found.
> > Then it falls back to model=basic.  Returning a negative means a fatal
> > error to stop.  Meanwhile, in the case of sigmatel, there is no
> > fallback, so it always stop.
> > 
> > The patch below adds the fallback to model=ref in case that no proper
> > auto-config is available.  Please give it a try.
> > 
> > 
> > Takashi
> > 
> > 
> 
> ok. thanks. in our backport, we don't have 'STAC_925x_REF' 
> defined....should we be using 'STAC_REF' instead? Also, the change that 
> introduced this problem into our 'beta' tree was:
> 
> @@ -855,14 +1137,14 @@ static int stac92xx_parse_auto_config(st
> 
>         if ((err = snd_hda_parse_pin_def_config(codec, &spec->autocfg, 
> NULL)) < 0)
>                 return err;
> -       if (! spec->autocfg.line_outs && ! spec->autocfg.hp_pin)
> +       if (! spec->autocfg.line_outs)
>                 return 0; /* can't find valid pin config */
> 
> 
> If i revert this single change, i avoid the oops as well....

Could you try the latest ALSA version at first?

Debugging different versions at the same time is confusing and another
possible cause of bugs.  If you need backport, try it later.


Takashi

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: stac92xx_parse_auto_config() can return 0 on error?
  2007-02-06 11:47   ` Takashi Iwai
  2007-02-06 16:53     ` Jason Baron
@ 2007-02-08 15:30     ` Takashi Iwai
  2007-02-08 16:34       ` Jason Baron
  1 sibling, 1 reply; 7+ messages in thread
From: Takashi Iwai @ 2007-02-08 15:30 UTC (permalink / raw)
  To: Tobin Davis; +Cc: Jason Baron, John Linville, alsa-devel

At Tue, 06 Feb 2007 12:47:52 +0100,
I wrote:
> 
> At Mon, 05 Feb 2007 14:29:53 -0800,
> Tobin Davis wrote:
> > 
> > On Mon, 2007-02-05 at 16:40 -0500, Jason Baron wrote:
> > 
> >     hi,
> >     
> >     I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
> >     because the 'imux' parameter pointer was NULL. I traced this back to 
> >     stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
> >     set, without having initialized the 'input_mux'.
> >     
> >     The oops does not happen on the upstream kernel on this hardware b/c 
> >     'line_outs' was set (additional code has been added since RHEL4 to set 
> >     it). However, it still appears to me, that if 'line_outs' is not set, we 
> >     should return an error code, and not 0.
> >     
> >     thanks,
> >     
> >     -Jason
> > 
> > Try this patch to see if it works with your alsa version.  It's a simple patch that
> > adds the error condition, and should be able to be applied to older alsa versions.
> > 
> > Summary:  add error for undetected line_outs.
> > 
> > This adds an error condition to stac92xx_parse_auto_config if line_outs is less than
> > zero.
> > 
> 
> line_outs won't be negative, so this situation can't happen.
> The problem is that stac92xx_parse_auto_config() returns "0" (which
> means successful) even if line_outs = 0 and thus nothing is
> configured.  Simply changing it to return -EINVAL would work. 
> 
> However, this is no best solution.  In the case of patch_realtek.c or
> others, the codec probe routine behaves slightly differently. 
> *_parse_auto_config() may return 0 if no ane configuration is found.
> Then it falls back to model=basic.  Returning a negative means a fatal
> error to stop.  Meanwhile, in the case of sigmatel, there is no
> fallback, so it always stop.
> 
> The patch below adds the fallback to model=ref in case that no proper
> auto-config is available.  Please give it a try.

Can anyone confirm whether the patch fixes the problem or not?
I'd like to put this into the upstream ASAP, so that it's surely
merged in 2.6.21...


thanks,

Takashi

> 
> 
> Takashi
> 
> 
> diff -r a42a457ae20c pci/hda/patch_sigmatel.c
> --- a/pci/hda/patch_sigmatel.c	Mon Feb 05 14:56:20 2007 +0100
> +++ b/pci/hda/patch_sigmatel.c	Tue Feb 06 12:45:36 2007 +0100
> @@ -1800,6 +1800,7 @@ static int patch_stac925x(struct hda_cod
>  	spec->board_config = snd_hda_check_board_config(codec, STAC_925x_MODELS,
>  							stac925x_models,
>  							stac925x_cfg_tbl);
> + again:
>  	if (spec->board_config < 0) {
>  		snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC925x, using BIOS defaults\n");
>  		err = stac92xx_save_bios_config_regs(codec);
> @@ -1825,6 +1826,15 @@ static int patch_stac925x(struct hda_cod
>  	spec->mixer = stac925x_mixer;
>  
>  	err = stac92xx_parse_auto_config(codec, 0x8, 0x7);
> +	if (!err) {
> +		if (spec->board_config < 0) {
> +			printk(KERN_WARNING "hda_codec: No auto-config is "
> +			       "available, default to model=ref\n");
> +			spec->board_config = STAC_925x_REF;
> +			goto again;
> +		}
> +		err = -EINVAL;
> +	}
>  	if (err < 0) {
>  		stac92xx_free(codec);
>  		return err;
> @@ -1850,6 +1860,7 @@ static int patch_stac922x(struct hda_cod
>  	spec->board_config = snd_hda_check_board_config(codec, STAC_922X_MODELS,
>  							stac922x_models,
>  							stac922x_cfg_tbl);
> + again:
>  	if (spec->board_config < 0) {
>  		snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC922x, "
>  			"using BIOS defaults\n");
> @@ -1875,6 +1886,15 @@ static int patch_stac922x(struct hda_cod
>  	spec->multiout.dac_nids = spec->dac_nids;
>  	
>  	err = stac92xx_parse_auto_config(codec, 0x08, 0x09);
> +	if (!err) {
> +		if (spec->board_config < 0) {
> +			printk(KERN_WARNING "hda_codec: No auto-config is "
> +			       "available, default to model=ref\n");
> +			spec->board_config = STAC_D945_REF;
> +			goto again;
> +		}
> +		err = -EINVAL;
> +	}
>  	if (err < 0) {
>  		stac92xx_free(codec);
>  		return err;
> @@ -1903,6 +1923,7 @@ static int patch_stac927x(struct hda_cod
>  	spec->board_config = snd_hda_check_board_config(codec, STAC_927X_MODELS,
>  							stac927x_models,
>  							stac927x_cfg_tbl);
> + again:
>  	if (spec->board_config < 0) {
>                  snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC927x, using BIOS defaults\n");
>  		err = stac92xx_save_bios_config_regs(codec);
> @@ -1945,6 +1966,15 @@ static int patch_stac927x(struct hda_cod
>  	spec->multiout.dac_nids = spec->dac_nids;
>  
>  	err = stac92xx_parse_auto_config(codec, 0x1e, 0x20);
> +	if (!err) {
> +		if (spec->board_config < 0) {
> +			printk(KERN_WARNING "hda_codec: No auto-config is "
> +			       "available, default to model=ref\n");
> +			spec->board_config = STAC_D965_REF;
> +			goto again;
> +		}
> +		err = -EINVAL;
> +	}
>  	if (err < 0) {
>  		stac92xx_free(codec);
>  		return err;
> @@ -1970,6 +2000,7 @@ static int patch_stac9205(struct hda_cod
>  	spec->board_config = snd_hda_check_board_config(codec, STAC_9205_MODELS,
>  							stac9205_models,
>  							stac9205_cfg_tbl);
> + again:
>  	if (spec->board_config < 0) {
>  		snd_printdd(KERN_INFO "hda_codec: Unknown model for STAC9205, using BIOS defaults\n");
>  		err = stac92xx_save_bios_config_regs(codec);
> @@ -2008,6 +2039,15 @@ static int patch_stac9205(struct hda_cod
>  			    AC_VERB_SET_GPIO_MASK, 0x00000001);
>  
>  	err = stac92xx_parse_auto_config(codec, 0x1f, 0x20);
> +	if (!err) {
> +		if (spec->board_config < 0) {
> +			printk(KERN_WARNING "hda_codec: No auto-config is "
> +			       "available, default to model=ref\n");
> +			spec->board_config = STAC_9205_REF;
> +			goto again;
> +		}
> +		err = -EINVAL;
> +	}
>  	if (err < 0) {
>  		stac92xx_free(codec);
>  		return err;
> 
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier.
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/alsa-devel
> 

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: stac92xx_parse_auto_config() can return 0 on error?
  2007-02-08 15:30     ` Takashi Iwai
@ 2007-02-08 16:34       ` Jason Baron
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Baron @ 2007-02-08 16:34 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: John Linville, alsa-devel



On Thu, 8 Feb 2007, Takashi Iwai wrote:

> At Tue, 06 Feb 2007 12:47:52 +0100,
> I wrote:
> > 
> > At Mon, 05 Feb 2007 14:29:53 -0800,
> > Tobin Davis wrote:
> > > 
> > > On Mon, 2007-02-05 at 16:40 -0500, Jason Baron wrote:
> > > 
> > >     hi,
> > >     
> > >     I just ran into a RHEL4 U5 beta kernel oops in snd_hda_input_mux_info(), 
> > >     because the 'imux' parameter pointer was NULL. I traced this back to 
> > >     stac92xx_parse_auto_config(), which can return 0, if 'line_outs' is not 
> > >     set, without having initialized the 'input_mux'.
> > >     
> > >     The oops does not happen on the upstream kernel on this hardware b/c 
> > >     'line_outs' was set (additional code has been added since RHEL4 to set 
> > >     it). However, it still appears to me, that if 'line_outs' is not set, we 
> > >     should return an error code, and not 0.
> > >     
> > >     thanks,
> > >     
> > >     -Jason
> > > 
> > > Try this patch to see if it works with your alsa version.  It's a simple patch that
> > > adds the error condition, and should be able to be applied to older alsa versions.
> > > 
> > > Summary:  add error for undetected line_outs.
> > > 
> > > This adds an error condition to stac92xx_parse_auto_config if line_outs is less than
> > > zero.
> > > 
> > 
> > line_outs won't be negative, so this situation can't happen.
> > The problem is that stac92xx_parse_auto_config() returns "0" (which
> > means successful) even if line_outs = 0 and thus nothing is
> > configured.  Simply changing it to return -EINVAL would work. 
> > 
> > However, this is no best solution.  In the case of patch_realtek.c or
> > others, the codec probe routine behaves slightly differently. 
> > *_parse_auto_config() may return 0 if no ane configuration is found.
> > Then it falls back to model=basic.  Returning a negative means a fatal
> > error to stop.  Meanwhile, in the case of sigmatel, there is no
> > fallback, so it always stop.
> > 
> > The patch below adds the fallback to model=ref in case that no proper
> > auto-config is available.  Please give it a try.
> 
> Can anyone confirm whether the patch fixes the problem or not?
> I'd like to put this into the upstream ASAP, so that it's surely
> merged in 2.6.21...
> 
> 
> thanks,
> 
> Takashi
> 


The upstream code works fine on the box that had the oops, i think due to
the 'fixup' code at the end of 'snd_hda_parse_pin_def_config()' which we
didn't have in RHEL4. In rhel4 the patch wouldn't compile, due to the
STAC_D945_REF, being not defined. However, i did verify that just 
returning and error code made the oops go away.

-Jason

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-02-08 16:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-05 21:40 stac92xx_parse_auto_config() can return 0 on error? Jason Baron
2007-02-05 22:29 ` Tobin Davis
2007-02-06 11:47   ` Takashi Iwai
2007-02-06 16:53     ` Jason Baron
2007-02-06 17:00       ` Takashi Iwai
2007-02-08 15:30     ` Takashi Iwai
2007-02-08 16:34       ` Jason Baron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.