From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: stac92xx_parse_auto_config() can return 0 on error? Date: Thu, 08 Feb 2007 16:30:58 +0100 Message-ID: References: <1170714593.28278.151.camel@localhost> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (ns2.suse.de [195.135.220.15]) by alsa.jcu.cz (ALSA's E-mail Delivery System) with ESMTP id E378F2FE for ; Thu, 8 Feb 2007 16:31:14 +0100 (MET) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@lists.sourceforge.net Errors-To: alsa-devel-bounces@lists.sourceforge.net To: Tobin Davis Cc: Jason Baron , John Linville , alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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