[PATCH] ad1848 and cs4231 busy loop replacement

[PATCH] ad1848 and cs4231 busy loop replacement

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

This patch replaces busy loop with msleep and removes
incorrect debug message.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---

This should fix Rene's problem with the timeout message he got in his new Aztech driver.

The problem is that the driver wait for busy bit to go high but it may not happen if the 
auto-calibration is not set. The result is incorrect debug message (that timeout happened).
It is also possible (in theory) to get this message in the ad1848-lib on a slow CPU.

The patch just wait a period around the period needed for the auto-calibration. If there is
no auto-calibration set, the driver just loses few miliseconds. This may happen only
during driver start as both drivers set auto-calibration. If the auto-calibration is set
we give CPU more time to do something else then move to next waiting loop (for
the auto-calibration bit to get cleared).

The msleep delay is calculated as rounded down time of waiting number of cycles (from
ad1848k and cs4231a specs) with the highest frequency (48kHz).

Rene, please test this patch on the cs4231 codec.

diff -urp linux-2.6.22/sound/isa/ad1848/ad1848_lib.c linux-2.6.23/sound/isa/ad1848/ad1848_lib.c
--- linux-2.6.22/sound/isa/ad1848/ad1848_lib.c	2007-09-08 23:53:42.958067597 +0200
+++ linux-2.6.23/sound/isa/ad1848/ad1848_lib.c	2007-09-08 23:57:31.255077502 +0200
@@ -229,17 +229,12 @@ static void snd_ad1848_mce_down(struct s
 		spin_unlock_irqrestore(&chip->reg_lock, flags);
 		return;
 	}
+
 	/* calibration process */
+	msleep(7);
+
+	snd_printd("(2) jiffies = %li\n", jiffies);
 
-	for (timeout = 500; timeout > 0 && (snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) == 0; timeout--);
-	if ((snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) == 0) {
-		snd_printd("mce_down - auto calibration time out (1)\n");
-		spin_unlock_irqrestore(&chip->reg_lock, flags);
-		return;
-	}
-#if 0
-	printk("(2) timeout = %i, jiffies = %li\n", timeout, jiffies);
-#endif
 	time = HZ / 4;
 	while (snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) {
 		spin_unlock_irqrestore(&chip->reg_lock, flags);
diff -urp linux-2.6.22/sound/isa/cs423x/cs4231_lib.c linux-2.6.23/sound/isa/cs423x/cs4231_lib.c
--- linux-2.6.22/sound/isa/cs423x/cs4231_lib.c	2007-09-08 23:53:29.005272473 +0200
+++ linux-2.6.23/sound/isa/cs423x/cs4231_lib.c	2007-09-08 23:56:52.824887490 +0200
@@ -334,19 +334,12 @@ void snd_cs4231_mce_down(struct snd_cs42
 	    !(chip->hardware & (CS4231_HW_CS4231_MASK | CS4231_HW_CS4232_MASK))) {
 		return;
 	}
-	snd_cs4231_busy_wait(chip);
 
 	/* calibration process */
+	msleep(2);
+
+	snd_printd("(2) jiffies = %li\n", jiffies);
 
-	for (timeout = 500; timeout > 0 && (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) == 0; timeout--)
-		udelay(10);
-	if ((snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) == 0) {
-		snd_printd("cs4231_mce_down - auto calibration time out (1)\n");
-		return;
-	}
-#if 0
-	printk("(2) timeout = %i, jiffies = %li\n", timeout, jiffies);
-#endif
 	/* in 10 ms increments, check condition, up to 250 ms */
 	timeout = 25;
 	while (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) {

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Rene Herman]

[-- Attachment #1: Type: text/plain, Size: 2562 bytes --]

On 09/09/2007 12:12 AM, Krzysztof Helt wrote:

> This should fix Rene's problem with the timeout message he got in his new
> Aztech driver.
> 
> The problem is that the driver wait for busy bit to go high but it may 
> not happen if the auto-calibration is not set. The result is incorrect 
> debug message (that timeout happened). It is also possible (in theory) to
> get this message in the ad1848-lib on a slow CPU.
> 
> The patch just wait a period around the period needed for the 
> auto-calibration. If there is no auto-calibration set, the driver just 
> loses few miliseconds. This may happen only during driver start as both 
> drivers set auto-calibration. If the auto-calibration is set we give CPU
> more time to do something else then move to next waiting loop (for the 
> auto-calibration bit to get cleared).

Yes, thank you, this appears to be a proper analysis. Unfortunately, I don't 
believe the patch itself is correct though:

> The msleep delay is calculated as rounded down time of waiting number of
> cycles (from ad1848k and cs4231a specs) with the highest frequency
> (48kHz).

As you say, the next loop is waiting for the auto-calibration bit to go down 
again, so here we should be waiting (the maximum time needed) for it to come 
up after dropping the MCE bit. You do a msleep(7), which I assume is derived 
(somehow...) from the 384 cycles mentioned in the spec, at 48kHz, but that's 
the time the auto-calibration bit will stay up after _having_ come up.

On page 24 of the AD1848K spec (2nd column, near top) we have:

   * Clear the Mode Change Enable (MCE) bit

   * The Autocalibrate-In-Progrss (ACI) bit will transition from LO to
     HI within 5 sample periods. [ ... ]

Which seems to be saying that we should be waiting for only 5 cycles at that 
point. With the slowest (!) possible sampling rate of 5.5125 kHz that would 
mean 907 us, or sligtly under 1 ms.

For the CS4231, the datasheet implies ACI should be up immediately upon 
dropping MCE (when auto-calibrating). On page 20 of the CS4231A sheet:

   3) Return from Mode Change Enable by resetting the MCE bit [ ... ]
   4) Wait until ACI cleared to proceed

As such, I've tested the below on CS4231A and there it seems to be working 
fine. Also delaying for 1ms in cs2431 would be fine by me as well...

> Rene, please test this patch on the cs4231 codec.

Given that the only difference is that I wait less, yours would've worked as 
well -- thanks for looking into this! And could you viceversa test this on 
AD1848 and perhaps resubmit if you agree?

Rene.


[-- Attachment #2: ad1848-mce_down.diff --]
[-- Type: text/plain, Size: 1961 bytes --]

diff --git a/sound/isa/ad1848/ad1848_lib.c b/sound/isa/ad1848/ad1848_lib.c
index 8094282..4d951ac 100644
--- a/sound/isa/ad1848/ad1848_lib.c
+++ b/sound/isa/ad1848/ad1848_lib.c
@@ -227,16 +227,14 @@ static void snd_ad1848_mce_down(struct snd_ad1848 *chip)
 		spin_unlock_irqrestore(&chip->reg_lock, flags);
 		return;
 	}
-	/* calibration process */
 
-	for (timeout = 500; timeout > 0 && (snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) == 0; timeout--);
-	if ((snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) == 0) {
-		snd_printd("mce_down - auto calibration time out (1)\n");
-		spin_unlock_irqrestore(&chip->reg_lock, flags);
-		return;
-	}
+	/*
+	 * If auto-calibrating, CALIB_IN_PROGRESS will be up within 5 sample
+	 * periods which at the slowest rate of 5.5125 kHz means ~ 907 us.
+	 */
+	mdelay(1);
 #if 0
-	printk("(2) timeout = %i, jiffies = %li\n", timeout, jiffies);
+	printk("(2) jiffies = %li\n", jiffies);
 #endif
 	time = HZ / 4;
 	while (snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) {
diff --git a/sound/isa/cs423x/cs4231_lib.c b/sound/isa/cs423x/cs4231_lib.c
index 914d77b..bd1affe 100644
--- a/sound/isa/cs423x/cs4231_lib.c
+++ b/sound/isa/cs423x/cs4231_lib.c
@@ -344,18 +344,8 @@ void snd_cs4231_mce_down(struct snd_cs4231 *chip)
 	    !(chip->hardware & (CS4231_HW_CS4231_MASK | CS4231_HW_CS4232_MASK))) {
 		return;
 	}
-	snd_cs4231_busy_wait(chip);
-
-	/* calibration process */
-
-	for (timeout = 500; timeout > 0 && (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) == 0; timeout--)
-		udelay(10);
-	if ((snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) == 0) {
-		snd_printd("cs4231_mce_down - auto calibration time out (1)\n");
-		return;
-	}
 #if 0
-	printk("(2) timeout = %i, jiffies = %li\n", timeout, jiffies);
+	printk("(2) jiffies = %li\n", jiffies);
 #endif
 	/* in 10 ms increments, check condition, up to 250 ms */
 	timeout = 25;

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Rene Herman]

On 09/09/2007 11:09 PM, Rene Herman wrote:

> Given that the only difference is that I wait less, yours would've 
> worked as well -- thanks for looking into this! And could you viceversa 
> test this on AD1848 and perhaps resubmit if you agree?

Oh, and I forgot to say (sigh) that I use a mdelay() and not an msleep() in 
the ad1848 one due to the 1 ms being short enough and didn't keep the 
snd_printd() because if one, then all please...

If you yourself insist on keeping a change like that split off, then doing 
that as a second patch ontop is best I guess but I suspect Takashi wouldn't 
mind trivial stuff like that anymore than I would -- we're not rule-driven 
robots...

Rene.

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Krzysztof Helt]

Rene wrote:

> As you say, the next loop is waiting for the auto-calibration bit to go down
> again, so here we should be waiting (the maximum time needed) for it to come
> up after dropping the MCE bit. You do a msleep(7), which I assume is derived
> (somehow...) from the 384 cycles mentioned in the spec, at 48kHz, but that's
> the time the auto-calibration bit will stay up after _having_ come up.
>
> On page 24 of the AD1848K spec (2nd column, near top) we have:
>
>    * Clear the Mode Change Enable (MCE) bit
>
>    * The Autocalibrate-In-Progrss (ACI) bit will transition from LO to
>      HI within 5 sample periods. [ ... ]
>
> Which seems to be saying that we should be waiting for only 5 cycles at that
> point. With the slowest (!) possible sampling rate of 5.5125 kHz that would
> mean 907 us, or sligtly under 1 ms.
>

It does not matter. If you have to wait for the start of the
auto-calibration then for the end of auto-calibration you may as well
wait for the end of the auto-calibration only (unless you want to
follow the auto-calibration to the very single clock pulse ;-). The
delay here is simple to assure that auto-calibration started (and
probably is already under way). It may be longer then the start but it
should not be longer than whole auto-calibration.
I simply tried to do as long msleep as possible to allow other tasks
using CPU and decrease number of waiting loop iterations.

I tested on the AD1848 before I posted it. The 7ms delay was never
enough for auto-calibration to complete. Even at 48kHz, tens of
waiting loops iterations happened on my SC-6000. Tens of waiting loops
is much less than 1 jiffy, and msleep(2) is 2 jiffies.
Och, and my AD1848 returns 0xA revision which means it is AD1848K
despite it is marked as AD1848 only.

> fine. Also delaying for 1ms in cs2431 would be fine by me as well...
>

I tested this on SPARC cs4231. The 2ms and 1ms gives the same results
which may be attributed to kernel timer resolution (2 jiffies for
msleep(1) and msleep(2)). It was enough for the whole autocalibration
(no waiting loop iterations at 48kHz). I got 2.8ms delay at 48kHz when
I calculated it from the specs. I will retest it with kernel set to
1000 Hz to confirm if the  2ms is too long (it should not be we are
below kernel timer resolution).

I will post SPARC cs4231 patch (with sleep and improved waiting) when
Takashi will say if the kernel or alsa version of the driver is
correct.

> > Rene, please test this patch on the cs4231 codec.
>
> Given that the only difference is that I wait less, yours would've worked as
> well -- thanks for looking into this! And could you viceversa test this on
> AD1848 and perhaps resubmit if you agree?
>

Thanks,
Krzysztof

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Rene Herman]

On 09/10/2007 09:08 AM, Krzysztof Helt wrote:

> Rene wrote:

>> Which seems to be saying that we should be waiting for only 5 cycles at
>> that point. With the slowest (!) possible sampling rate of 5.5125 kHz 
>> that would mean 907 us, or sligtly under 1 ms.
>> 
> It does not matter. If you have to wait for the start of the 
> auto-calibration then for the end of auto-calibration you may as well 
> wait for the end of the auto-calibration only (unless you want to follow 
> the auto-calibration to the very single clock pulse ;-). The delay here 
> is simple to assure that auto-calibration started (and probably is 
> already under way). It may be longer then the start but it should not be 
> longer than whole auto-calibration.
> 
> I simply tried to do as long msleep as possible to allow other tasks 
> using CPU and decrease number of waiting loop iterations.

Well, no, sorry, but I consider that to be completely breaking the logic of 
the code. One line after this bit, we're doing the "wait for calibration to 
finish" loop (and already schedule ourselves away while doing so, for 250ms 
no less) meaning that at this point we should only wait long enough to be 
guaranteed that the ACI bit reflects reality -- those 5 cycles.

Your: wait unconditionally until calibration _nearly_ done, then go wait for 
it for 250 ms to be really done.

Mine: wait unconditionally until calibration has started, then go wait for 
it for 250 ms to finish.

Really -- please just do the 1 ms delay for ad1848 and either no delay or 
the same 1 ms for cs4231 if you want to keep them in sync (and the comment 
explaining why it's 1 ms). I don't particularly care about mdelay versus 
msleep here (although a mere 1 ms fits mdelay better) but your setup is 
pointlessly obscuring the code-flow.

Rene.

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Krzysztof Helt]

On 9/10/07, Rene Herman <rene.herman@gmail.com> wrote:
> On 09/10/2007 09:08 AM, Krzysztof Helt wrote:
>
> > Rene wrote:

> Well, no, sorry, but I consider that to be completely breaking the logic of
> the code.

No I changed the logic of this code not to wait for specifically for
callibration "start" but into calibration "under way".

> One line after this bit, we're doing the "wait for calibration to
> finish" loop (and already schedule ourselves away while doing so, for 250ms
> no less) meaning that at this point we should only wait long enough to be
> guaranteed that the ACI bit reflects reality -- those 5 cycles.
>
> Your: wait unconditionally until calibration _nearly_ done, then go wait for
> it for 250 ms to be really done.
>
> Mine: wait unconditionally until calibration has started, then go wait for
> it for 250 ms to finish.
>

This discussion get me amused. Look at the code:

Mine:

msleep(7); /* or msleep(2) for CS4231 */

/*waiting loop 250ms*/
while ...

Yours:
msleep(1); /* or msleep(1) for CS4231 */

/*waiting loop 250ms */
while ...

So the only difference is 6 (or 1) ms and this time will be spent in
the loop anyway. Are we arguing 1ms (for CS4231) in 250ms waiting
loop?
If you really want it can be substracted from the waiting loop 250ms
to be exactly the same as it was.

> Really -- please just do the 1 ms delay for ad1848 and either no delay or
> the same 1 ms for cs4231 if you want to keep them in sync

I don't understand "keep them in sync". After the patch they behave
the same but with different delay (they have different speed). It can
be 2ms (as for faster one) for both but I wanted to avoid doing many
loop iterations if possible.

Regards,
Krzysztof

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Rene Herman]

On 09/10/2007 02:42 PM, Krzysztof Helt wrote:

> On 9/10/07, Rene Herman <rene.herman@gmail.com> wrote:

>> Well, no, sorry, but I consider that to be completely breaking the logic of
>> the code.
> 
> No I changed the logic of this code not to wait for specifically for
> callibration "start" but into calibration "under way".

No, waiting for calibration the be under way is what my 0/1 ms does. You are 
waiting for it to be nearly done, which is complete nonsense. One line below 
we are waiting for 250 ms (generally with _one_ pass through the loop -- we 
only wake up through signals) anyway!

The no delay at all from cs4231 is the logic -- when we've dropped MCE, ACI 
comes up (when auto-calibrating) and we only wait for it to finish. For 
ad1848, ACI up may take 5 cycles from MCE down so we delay 1 ms so we know 
we're testing correctly.

>> Your: wait unconditionally until calibration _nearly_ done, then go wait for
>> it for 250 ms to be really done.
>>
>> Mine: wait unconditionally until calibration has started, then go wait for
>> it for 250 ms to finish.

[ ... ]

> So the only difference is 6 (or 1) ms and this time will be spent in
> the loop anyway. Are we arguing 1ms (for CS4231) in 250ms waiting
> loop?

No, we are arguing maintaining code. Do not obscure the code flow for no 
reason. Fix your logic or (for what it's worth) I am going to NAK the change.

> I don't understand "keep them in sync".

In sync source-code wise. While the no delay from cs4231 may be the rule, 
ad1848 needs a small delay so if you'd wanted to keep them looking the same 
I wouldn't care about a 1 ms delay for cs4231 as well. If you don't, fine as 
well.

Rene.

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Krzysztof Helt]

On Mon, 10 Sep 2007 15:13:24 +0200
Rene Herman <rene.herman@gmail.com> wrote:

> On 09/10/2007 02:42 PM, Krzysztof Helt wrote:
> 
> > On 9/10/07, Rene Herman <rene.herman@gmail.com> wrote:
> 
> >> Well, no, sorry, but I consider that to be completely breaking the logic of
> >> the code.
> > 
> > No I changed the logic of this code not to wait for specifically for
> > callibration "start" but into calibration "under way".
> 
> No, waiting for calibration the be under way is what my 0/1 ms does. You are 
> waiting for it to be nearly done, which is complete nonsense. One line below 
> we are waiting for 250 ms (generally with _one_ pass through the loop -- we 
> only wake up through signals) anyway!
> 

I don't buy your argument that it is complete nonsense.
I want some merit argument like:
your code uses more memory/cpu, 
your code is longer,
your code drives hardware into undefined states,
your code push hardware outside hardware limits,
your code is slower/adds more delay.

Also, I don't think mimicking the hardware behaviour in driver code is 
correct paradigm. I think that correct way is to _drive_ the hardware. If the
hardware must be taken from state A to state C and goes through state B
, the driver may wait only for the C completely ignoring the B if the B
is irrelevant (no need to set or get something from the hardware until the C).

Another thing I believe is that driver should be "CPU friendly" means it should
gives back control to the CPU every time it knows the hardware is busy
and nothing can be done for some known period of time. That's why I don't
want to use all these m/udelay functions. They make CPU busy. Sometimes
they are must have, sometimes not. If one knows that it must wait at least
7ms why it should wait 1ms than makes another 6ms inside the loop?

Actually, if you look into the lower delay argument, the waiting loop after 
the change is inefficient. The whole calibration takes 2-3ms at the highest 
rate, but one has to wait in 10ms blocks. In order to lower this delay one 
must change it to some finer way of returning time to the CPU (cpu_relax()
or schedule_timeout() - like ad1848). If it is done, the loop will make 
thousands of iterations every few miliseconds. Taking your argument 
that loop should be passed in the least possible iterations (you wrote 
"preferably one pass"), the only way is to wait longer before entering the loop.

> 
> No, we are arguing maintaining code. Do not obscure the code flow for no 
> reason. Fix your logic or (for what it's worth) I am going to NAK the change.
> 

The code does not be obscured with one simple comment before msleep: 
/* wait till calibration is nearly done */

Also comments like "my way or highway" (NAK) won't make you many friends.

Krzysztof

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Krzysztof Helt]

On Sun, 09 Sep 2007 23:19:56 +0200
Rene Herman <rene.herman@gmail.com> wrote:

> On 09/09/2007 11:09 PM, Rene Herman wrote:
> 
> > Given that the only difference is that I wait less, yours would've 
> > worked as well -- thanks for looking into this! And could you viceversa 
> > test this on AD1848 and perhaps resubmit if you agree?
> 
> Oh, and I forgot to say (sigh) that I use a mdelay() and not an msleep() in 
> the ad1848 one due to the 1 ms being short enough and didn't keep the 
> snd_printd() because if one, then all please...
> 

I tested CS4231 on 450MHz sparc with 1000Hz kernel (I don't have PC card yet).
The waiting loop was changed as in my second patch (the 10ms blocks 
to schedule_timeout()). I played 48kHz wav (alsa "Front_Center.wav").

The waiting loop was passed 340 times after mce was down with msleep(1).
The delay between snd_printd "2 jiffies" and "8 jiffies" was 3 jiffies.

The waiting loop was passed 34 times after mce was down with msleep(2).
The delay between snd_printd "2 jiffies" and "8 jiffies" was 3 jiffies too.

It took about ten times less loop passes if the driver waits 2ms.
The 2ms seems like the optimal value for the CS4231.

Regards,
Krzysztof

Re: [PATCH] ad1848 and cs4231 busy loop replacement

[Rene Herman]

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

On 09/10/2007 07:05 PM, Krzysztof Helt wrote:

> Rene Herman <rene.herman@gmail.com> wrote:

>> No, waiting for calibration the be under way is what my 0/1 ms does.
>> You are waiting for it to be nearly done, which is complete nonsense.
>> One line below we are waiting for 250 ms (generally with _one_ pass
>> through the loop -- we only wake up through signals) anyway!
> 
> I don't buy your argument that it is complete nonsense.

You don't need to buy it -- I presented it to you for free on a fucking 
platter 4 times now. You are making me waste my time. Answer this or just 
bury this useles nonsense discussion: why do you insist on delaying for 7 ms 
when the specification says (and the hardware is confirming) that 1 ms is 
enough?

You are delaying 7 ms and then 250 ms when we are auto-calibrating. I delay 
1 ms and then 250 ms when we are auto-calibrating. Why do you delay longer 
than the hardware needs and make everyone who ever reads this code wonder 
why the hell that odd delay is in there? And make everyone who ever will be 
re-writing or porting this code drag it along on the notion that someone, 
somewhere probably once had a purpose for it?

Fix it up with a comment? Why? There's still not a single point to it.

Why 7? Or 2? Or whatever more time than needed for the calibration to start?

> Also comments like "my way or highway" (NAK) won't make you many friends.

Good, I don't need more friends. You are doing the generic newbie thing 
where you assume you obviously must know everything better than everyone 
else. Just shelve that dumb crap.

You also may want to take into account that I'm taking the time to review 
your code and dig trough datasheets while I had in fact different things to 
do and it's proving utterly useless.

Fixed patch attached.

Rene.

[-- Attachment #2: mce_down.diff --]
[-- Type: text/plain, Size: 2226 bytes --]

diff --git a/sound/isa/ad1848/ad1848_lib.c b/sound/isa/ad1848/ad1848_lib.c
index 8094282..dcd4435 100644
--- a/sound/isa/ad1848/ad1848_lib.c
+++ b/sound/isa/ad1848/ad1848_lib.c
@@ -227,16 +227,15 @@ static void snd_ad1848_mce_down(struct snd_ad1848 *chip)
 		spin_unlock_irqrestore(&chip->reg_lock, flags);
 		return;
 	}
-	/* calibration process */
 
-	for (timeout = 500; timeout > 0 && (snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) == 0; timeout--);
-	if ((snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) == 0) {
-		snd_printd("mce_down - auto calibration time out (1)\n");
-		spin_unlock_irqrestore(&chip->reg_lock, flags);
-		return;
-	}
+        /*
+         * Wait for (possible -- during init auto-calibration may not be set)
+         * calibration process to start. Needs upto 5 sample periods on AD1848
+         * which at the slowest possible rate of 5.5125 kHz means 907 us.
+         */
+        msleep(1);
 #if 0
-	printk("(2) timeout = %i, jiffies = %li\n", timeout, jiffies);
+	printk("(2) jiffies = %li\n", jiffies);
 #endif
 	time = HZ / 4;
 	while (snd_ad1848_in(chip, AD1848_TEST_INIT) & AD1848_CALIB_IN_PROGRESS) {
diff --git a/sound/isa/cs423x/cs4231_lib.c b/sound/isa/cs423x/cs4231_lib.c
index 914d77b..9e8e0f1 100644
--- a/sound/isa/cs423x/cs4231_lib.c
+++ b/sound/isa/cs423x/cs4231_lib.c
@@ -346,16 +346,14 @@ void snd_cs4231_mce_down(struct snd_cs4231 *chip)
 	}
 	snd_cs4231_busy_wait(chip);
 
-	/* calibration process */
-
-	for (timeout = 500; timeout > 0 && (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) == 0; timeout--)
-		udelay(10);
-	if ((snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) == 0) {
-		snd_printd("cs4231_mce_down - auto calibration time out (1)\n");
-		return;
-	}
+	/*
+	 * Wait for (possible -- during init auto-calibration may not be set)
+	 * calibration process to start. Needs upto 5 sample periods on AD1848
+	 * which at the slowest possible rate of 5.5125 kHz means 907 us.
+	 */
+	msleep(1);
 #if 0
-	printk("(2) timeout = %i, jiffies = %li\n", timeout, jiffies);
+	printk("(2) jiffies = %li\n", jiffies);
 #endif
 	/* in 10 ms increments, check condition, up to 250 ms */
 	timeout = 25;

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel