From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755556Ab1CGJza (ORCPT ); Mon, 7 Mar 2011 04:55:30 -0500 Received: from cantor.suse.de ([195.135.220.2]:37845 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755236Ab1CGJz2 (ORCPT ); Mon, 7 Mar 2011 04:55:28 -0500 Date: Mon, 07 Mar 2011 10:55:26 +0100 Message-ID: From: Takashi Iwai To: Jesper Juhl Cc: Paul Bolle , Jaroslav Kysela , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] intel8x0m: wait a bit before warm reset check In-Reply-To: References: <1299366106.2470.5.camel@t41.thuisdomein> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) 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 List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Sun, 6 Mar 2011 01:12:26 +0100 (CET), Jesper Juhl wrote: > > On Sun, 6 Mar 2011, Paul Bolle wrote: > > > At every resume a laptop I use prints this message (at KERN_ERR level): > > ALSA sound/pci/intel8x0m.c:904: AC'97 warm reset still in progress? [0x2] > > > > The thing to note here is that 0x2 corresponds to ICH_AC97COLD. Ie, what > > seems to be happening is that the register involved indicated a warm > > reset for some time (as the ICH_AC97WARM bit was set) but by the time > > the warning is printed, and that same register is checked again, that > > bit is already cleared and only the ICH_AC97COLD bit is still set. > > > > It turns out a warm reset needs some time to settle, but it is currently > > checked right away. The test therefore fails the first time it is done > > and schedule_timeout_uninterruptible() will be called. Once we return > > from that jiffies is already (far) past end_time on this laptop, so we > > exit the loop, print a warning, and exit the function while the warm > > reset actually succeeded. > > > > One way to fix this is to call udelay() after writing to the register > > involved. A handful of tests suggests 500 usecs is a safe value. (This > > might punish the "finish cold reset" case, but on this laptop such a > > cold reset apparently never happens, so I can't say for sure.) > > > > While we're at it drop the extra single tick from end_time, as it looks > > rather silly. > > > > Signed-off-by: Paul Bolle > > --- > > 0) v1 was called " intel8x0m: schedule timeout before warm reset check". > > > > 1) A udelay of 250 usecs always failed while 300 usecs always worked. So > > 500 usecs might be a bit cautious. > > > > Wouldn't it be nice to put these notes in a comment in the code so that > future readers don't have to scratch their heads and wonder where the > udelay() with some magical constant comes from? That'd be better, indeed. > Also, if udelay(300) always worked in your tests, wouldn't udelay(400) be > more than enough to provide a nice fat safety margin (33% more than > "always works")? Well... I think you know how meaningless to quote a percentage representation in such a case... > Are there no official documents that specify how long > this is expected to take that we could base this delay on instead of just > testing on one system? Heh, can you really trust such a document? ;) It's basically a workaround for a specific codec chip that doesn't set the bit reflecting to the right status after resume/power-up. That is, it's just for the codec chip on Paul's machine. Others work well without this delay. Also, as mentioned earlier, it's better to use usleep_range() than udelay(). There is no hard upper-limit in this case, and any longer sleep works. thanks, Takashi