ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

Modify loops in such way that the register value is checked also after the timeout condition, just in case the heavy interrupt load etc. caused the thread to sleep for the time period exceeding the timeout value.

While at it remove an extra ALI_STIMER read from snd_ali_stimer_ready().

Reported-by Jack Byer <[email protected]>

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
 sound/pci/ali5451/ali5451.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: b/sound/pci/ali5451/ali5451.c
===================================================================

At Wed, 19 Aug 2009 22:09:50 +0200,

Quoted 21 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

Modify loops in such way that the register value is checked also after the timeout condition, just in case the heavy interrupt load etc. caused the thread to sleep for the time period exceeding the timeout value.

While at it remove an extra ALI_STIMER read from snd_ali_stimer_ready().

Reported-by Jack Byer <[email protected]>

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
 sound/pci/ali5451/ali5451.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: b/sound/pci/ali5451/ali5451.c
===================================================================

sound/pci/ali5451/ali5451.c static int snd_ali_codec_ready(struct sn · line 320

Quoted 8 lines

sound/pci/ali5451/ali5451.c +3 -1

@@ -314,8 +314,11 @@ static int snd_ali_codec_ready(struct sn
 		res = snd_ali_5451_peek(codec,port);
 		if (!(res & 0x8000))
 			return 0;
+		if (!time_after_eq(end_time, jiffies))
+			break;
 		schedule_timeout_uninterruptible(1);
-	} while (time_after_eq(end_time, jiffies));
+	} while (1);

Using for (;;) is more generic. I see your patch keeps the changes minimal, but I'm afraid that the result, do {} while(1), can be misleading.

Could you replace with for (;;) ?

Quoted 36 lines · On Thursday 20 August 2009 08:33:21 Takashi Iwai wrote:

At Wed, 19 Aug 2009 22:09:50 +0200,

Quoted 21 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

Modify loops in such way that the register value is checked also after the timeout condition, just in case the heavy interrupt load etc. caused the thread to sleep for the time period exceeding the timeout value.

While at it remove an extra ALI_STIMER read from snd_ali_stimer_ready().

Reported-by Jack Byer <[email protected]>

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
 sound/pci/ali5451/ali5451.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: b/sound/pci/ali5451/ali5451.c
===================================================================

sound/pci/ali5451/ali5451.c static int snd_ali_codec_ready(struct sn · line 320

Quoted 8 lines

sound/pci/ali5451/ali5451.c +3 -1

@@ -314,8 +314,11 @@ static int snd_ali_codec_ready(struct sn
 		res = snd_ali_5451_peek(codec,port);
 		if (!(res & 0x8000))
 			return 0;
+		if (!time_after_eq(end_time, jiffies))
+			break;
 		schedule_timeout_uninterruptible(1);
-	} while (time_after_eq(end_time, jiffies));
+	} while (1);

Using for (;;) is more generic. I see your patch keeps the changes minimal, but I'm afraid that the result, do {} while(1), can be misleading.

Quoted 1 line

Could you replace with for (;;) ?

Sure, though I don't see a much value added from this operation..

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH v2] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()

Modify loops in such way that the register value is checked also after the timeout condition, just in case the heavy interrupt load etc. caused the thread to sleep for the time period exceeding the timeout value.

While at it remove an extra ALI_STIMER read from snd_ali_stimer_ready().

Reported-by Jack Byer <[email protected]>

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
 sound/pci/ali5451/ali5451.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Index: b/sound/pci/ali5451/ali5451.c
===================================================================

sound/pci/ali5451/ali5451.c +12 -6

@@ -310,12 +310,16 @@ static int snd_ali_codec_ready(struct sn
 	unsigned int res;
 
 	end_time = jiffies + msecs_to_jiffies(250);
-	do {
+
+	for (;;) {
 		res = snd_ali_5451_peek(codec,port);
 		if (!(res & 0x8000))
 			return 0;
+		if (!time_after_eq(end_time, jiffies))
+			break;
 		schedule_timeout_uninterruptible(1);
-	} while (time_after_eq(end_time, jiffies));
+	}
+
 	snd_ali_5451_poke(codec, port, res & ~0x8000);
 	snd_printdd("ali_codec_ready: codec is not ready.\n ");
 	return -EIO;

@@ -327,15 +331,17 @@ static int snd_ali_stimer_ready(struct s
 	unsigned long dwChk1,dwChk2;
 
 	dwChk1 = snd_ali_5451_peek(codec, ALI_STIMER);
-	dwChk2 = snd_ali_5451_peek(codec, ALI_STIMER);
-
 	end_time = jiffies + msecs_to_jiffies(250);
-	do {
+
+	for (;;) {
 		dwChk2 = snd_ali_5451_peek(codec, ALI_STIMER);
 		if (dwChk2 != dwChk1)
 			return 0;
+		if (!time_after_eq(end_time, jiffies))
+			break;
 		schedule_timeout_uninterruptible(1);
-	} while (time_after_eq(end_time, jiffies));
+	}
+
 	snd_printk(KERN_ERR "ali_stimer_read: stimer is not ready.\n");
 	return -EIO;
 }