Date: Tue, 08 Jul 2008 12:16:35 +0200
Message-ID: <s5h1w24vfy4.wl%tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Thadeu Lima de Souza Cascardo <cascardo@minaslivre.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Wait for all codecs to be ready if doing a cold reset
In-Reply-To: <20080707173654.GA28388@vespa.holoscopio.com>
References: <20080706171548.GA9962@vespa.holoscopio.com>
	<s5hfxqlvec2.wl%tiwai@suse.de>
	<20080707173654.GA28388@vespa.holoscopio.com>
User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 Emacs/22.2
 (x86_64-suse-linux-gnu) MULE/5.0 (SAKAKI)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3840
Lines: 107

At Mon, 7 Jul 2008 14:36:55 -0300,
Thadeu Lima de Souza Cascardo wrote:
> 
> On Mon, Jul 07, 2008 at 06:39:09PM +0200, Takashi Iwai wrote:
> > At Sun, 6 Jul 2008 14:15:56 -0300,
> > Thadeu Lima de Souza Cascardo wrote:
> > > 
> > > If AC97_POWER_SAVE is enabled, intel8x0 does a cold reset when
> > > initializing the codecs. While resuming, it does not wait for all codecs
> > > to be active. Sound card does not work after a resume without it,
> > > however. This patch fixes it.
> > 
> > Thanks for the patch.
> > 
> > But, I still don't figure out why this is needed.
> > In the else block (with the comment "resume phase"), you can find the
> > loop to wait for the all *probed* codecs.  The difference with the
> > code you moved is that it checks only the bits corresponding to the
> > already probed codecs.  In other words, the other bits should be
> > irrelevant with the hardware.
> > 
> > I guess it's not about the loop but the additional 1/4 sec delay that
> > did fix the resume on your device.  Please check how is the status
> > bits and whether it passed the loop in the middle.
> > 
> > 
> > Takashi
> > 
> > 
> 
> The 1/4 sec delay came in my mind as one of the possible reasons, and
> that's why I've made some tests. status and nstatus are 0x200, while
> codec_isr_bits is 0x300. The loop waits for the status register give us
> 0x300 as the active codecs, instead of the only one probed. Since the
> cold reset in the case of a power saving cleans up all codec registers,
> it is needed that we wait for all codecs again (like in the probe case).

You loaded the modem driver as well?
If so, what happens if you unload modem driver?


thanks,

Takashi


> If you need more information or that I ran any more tests, I'd be glad
> to do it. By the way, I've tested the patch against your audio and modem
> merged version and it works OK.
>
> Best Regards,
> Thadeu Cascardo.
> 
> > > ---
> > >  sound/pci/intel8x0.c |   24 ++++++++++++++----------
> > >  1 files changed, 14 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
> > > index 048d99e..7228a0a 100644
> > > --- a/sound/pci/intel8x0.c
> > > +++ b/sound/pci/intel8x0.c
> > > @@ -2335,16 +2335,6 @@ static int snd_intel8x0_ich_chip_init(struct intel8x0 *chip, int probing)
> > >  				   igetdword(chip, ICHREG(GLOB_STA)));
> > >  			return -EIO;
> > >  		}
> > > -
> > > -		/* wait for other codecs ready status. */
> > > -		end_time = jiffies + HZ / 4;
> > > -		while (status != chip->codec_isr_bits &&
> > > -		       time_after_eq(end_time, jiffies)) {
> > > -			schedule_timeout_uninterruptible(1);
> > > -			status |= igetdword(chip, ICHREG(GLOB_STA)) &
> > > -				chip->codec_isr_bits;
> > > -		}
> > > -
> > >  	} else {
> > >  		/* resume phase */
> > >  		int i;
> > > @@ -2363,6 +2353,20 @@ static int snd_intel8x0_ich_chip_init(struct intel8x0 *chip, int probing)
> > >  		} while (time_after_eq(end_time, jiffies));
> > >  	}
> > >  
> > > +#ifndef CONFIG_SND_AC97_POWER_SAVE
> > > +	if (probing)
> > > +#endif
> > > +	{
> > > +		/* wait for other codecs ready status. */
> > > +		end_time = jiffies + HZ / 4;
> > > +		while (status != chip->codec_isr_bits &&
> > > +		       time_after_eq(end_time, jiffies)) {
> > > +			schedule_timeout_uninterruptible(1);
> > > +			status |= igetdword(chip, ICHREG(GLOB_STA)) &
> > > +				chip->codec_isr_bits;
> > > +		}
> > > +	}
> > > +
> > >  	if (chip->device_type == DEVICE_SIS) {
> > >  		/* unmute the output on SIS7012 */
> > >  		iputword(chip, 0x4c, igetword(chip, 0x4c) | 1);
> > > -- 
> > > 1.5.6
> > > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/