Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Rene Herman <rene.herman@gmail.com>
To: Trent Piepho <xyzzy@speakeasy.org>
Cc: Krzysztof Helt <krzysztof.h1@gmail.com>,
	Takashi Iwai <tiwai@suse.de>, devel <alsa-devel@alsa-project.org>
Subject: Re: [PATCH] ad1838/cs4231 -- fix MCE timeout upon initial load
Date: Wed, 19 Sep 2007 04:48:28 +0200	[thread overview]
Message-ID: <46F08DFC.7000803@gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.58.0709181634550.27377@shell4.speakeasy.net>

On 09/19/2007 03:27 AM, Trent Piepho wrote:

> On the other hand, cpu_relax() is totally different.  It's basically a
> hint to the processor to lower the clock speed/power consumption or give 
> resources to a sibling virtual processor in hyper-threading.  i.e., it's
> ok to call cpu_relax() from an interrupt or while atomic.
> 
> Might be a good idea in the non-sleeping busy loops, like the one in 
> snd_ad1848_wait(), but I bet the udelay() already does that.

It does (when using the TSC).

>> The original idea was to make this first waiting longer than loop
>> granularity. 7ms before the loop than 1ms granularity inside the loop. 
>> Rene Herman was against it so he changed it to be the same as 1ms
>> inside the loop.

Well, at that time, it still was first the 1 and then a full 250 -- or at 
least in my 2.6.22.x schedule_timeout_interruptible() version that actually 
scheduled...

> With the delay time being dependent on sample rate, and with different 
> chips being considerably faster than others, picking a good value for the
> first iteration is complicated.  If one could avoid 70 ms of busy
> looping in the kernel by doing so, then that might be worth it.  But we
> don't busy loop, just poll once per ms (even less when HZ<1000), so it
> seems that there is very little to gain here to justify the extra
> complexity.

Quite -- I plan on being around sound/isa/ for some time and am as such also 
already concerned with maintainability. If there's one thing I do know, it's 
that obviousness is an extremely important factor in that. So if the 
datasheet and hardware say 5 sample periods are enough, so be they...

>> Break this long line. You may calculate "regsel &
>> AD1848_CALIB_IN_PROGRESS"
> 
> Good idea, I've done that.

No goto? :-)

Rene.

  reply	other threads:[~2007-09-19  2:49 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-10 18:29 [PATCH] ad1838/cs4231 -- fix MCE timeout upon initial load Rene Herman
2007-09-10 21:40 ` Krzysztof Helt
2007-09-10 21:37   ` Takashi Iwai
2007-09-10 21:43   ` Rene Herman
2007-09-10 21:45     ` Rene Herman
2007-09-11  8:56     ` Krzysztof Helt
2007-09-11 20:42       ` Rene Herman
2007-09-17 21:04 ` Trent Piepho
2007-09-17 21:47   ` Krzysztof Helt
2007-09-18  1:18     ` Trent Piepho
2007-09-18  7:20       ` Krzysztof Helt
2007-09-19  1:27         ` Trent Piepho
2007-09-19  2:48           ` Rene Herman [this message]
2007-09-18  0:17   ` Rene Herman
2007-09-18  1:57     ` Rene Herman
2007-09-18  4:24       ` Rene Herman
2007-09-18 11:03         ` Rene Herman
2007-09-18 11:54         ` Takashi Iwai
2007-09-18 13:38           ` [ PATCH 1/4] alsa-kernel: schedule_timeout() fixes Rene Herman
2007-09-18 16:39             ` Takashi Iwai
2007-09-18 13:49           ` [PATCH 2/4] alsa-kernel: schedule_timeout() fix for kernel/drivers/input/touchscreen/ucb1400_ts.c Rene Herman
2007-09-18 13:58             ` Takashi Iwai
2007-09-18 13:58               ` Rene Herman
2007-09-18 13:57           ` [PATCH] ad1838/cs4231 -- fix MCE timeout upon initial load Rene Herman
2007-09-18 14:27             ` [PATCH 3/4] alsa-kernel: schedule_timeout() fix for core/seq/seq_instr.c Rene Herman
2007-09-18 16:38             ` [PATCH] ad1838/cs4231 -- fix MCE timeout upon initial load Takashi Iwai
2007-09-18 14:34           ` [PATCH 4/4] alsa-driver: schedule_timeout() fixes Rene Herman
2007-09-18 16:44             ` Takashi Iwai
2007-09-18 17:05               ` Rene Herman
2007-09-18 17:09                 ` Rene Herman
2007-09-18 17:22                   ` Takashi Iwai
2007-09-18  2:32     ` [PATCH] ad1838/cs4231 -- fix MCE timeout upon initial load Trent Piepho
2007-09-18  6:50       ` Rene Herman
2007-09-18  7:33       ` Krzysztof Helt
2007-09-18  8:02       ` Krzysztof Helt
2007-09-18  8:17         ` Rene Herman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46F08DFC.7000803@gmail.com \
    --to=rene.herman@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=krzysztof.h1@gmail.com \
    --cc=tiwai@suse.de \
    --cc=xyzzy@speakeasy.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox