Re: stac92xx_parse_auto_config() can return 0 on error?

[Takashi Iwai <tiwai@suse.de>]

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