* [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()
@ 2009-08-19 20:09 Bartlomiej Zolnierkiewicz
2009-08-20 6:33 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-08-19 20:09 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Jack Byer, alsa-devel, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
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 <ojbyer@usa.net>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
sound/pci/ali5451/ali5451.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
Index: b/sound/pci/ali5451/ali5451.c
===================================================================
--- a/sound/pci/ali5451/ali5451.c
+++ b/sound/pci/ali5451/ali5451.c
@@ -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);
+
snd_ali_5451_poke(codec, port, res & ~0x8000);
snd_printdd("ali_codec_ready: codec is not ready.\n ");
return -EIO;
@@ -327,15 +330,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 {
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));
+ } while (1);
+
snd_printk(KERN_ERR "ali_stimer_read: stimer is not ready.\n");
return -EIO;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()
2009-08-19 20:09 [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready() Bartlomiej Zolnierkiewicz
@ 2009-08-20 6:33 ` Takashi Iwai
2009-08-23 13:27 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2009-08-20 6:33 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Jack Byer, alsa-devel, linux-kernel
At Wed, 19 Aug 2009 22:09:50 +0200,
Bartlomiej Zolnierkiewicz wrote:
>
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 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 <ojbyer@usa.net>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
> sound/pci/ali5451/ali5451.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> Index: b/sound/pci/ali5451/ali5451.c
> ===================================================================
> --- a/sound/pci/ali5451/ali5451.c
> +++ b/sound/pci/ali5451/ali5451.c
> @@ -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 (;;) ?
thanks,
Takashi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()
2009-08-20 6:33 ` Takashi Iwai
@ 2009-08-23 13:27 ` Bartlomiej Zolnierkiewicz
2009-08-23 17:00 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-08-23 13:27 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Jack Byer, alsa-devel, linux-kernel
On Thursday 20 August 2009 08:33:21 Takashi Iwai wrote:
> At Wed, 19 Aug 2009 22:09:50 +0200,
> Bartlomiej Zolnierkiewicz wrote:
> >
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > 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 <ojbyer@usa.net>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> > sound/pci/ali5451/ali5451.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > Index: b/sound/pci/ali5451/ali5451.c
> > ===================================================================
> > --- a/sound/pci/ali5451/ali5451.c
> > +++ b/sound/pci/ali5451/ali5451.c
> > @@ -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 (;;) ?
Sure, though I don't see a much value added from this operation..
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
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 <ojbyer@usa.net>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
sound/pci/ali5451/ali5451.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
Index: b/sound/pci/ali5451/ali5451.c
===================================================================
--- a/sound/pci/ali5451/ali5451.c
+++ b/sound/pci/ali5451/ali5451.c
@@ -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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready()
2009-08-23 13:27 ` Bartlomiej Zolnierkiewicz
@ 2009-08-23 17:00 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2009-08-23 17:00 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Jack Byer, alsa-devel, linux-kernel
At Sun, 23 Aug 2009 15:27:25 +0200,
Bartlomiej Zolnierkiewicz wrote:
>
> On Thursday 20 August 2009 08:33:21 Takashi Iwai wrote:
> > At Wed, 19 Aug 2009 22:09:50 +0200,
> > Bartlomiej Zolnierkiewicz wrote:
> > >
> > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > 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 <ojbyer@usa.net>
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > ---
> > > sound/pci/ali5451/ali5451.c | 11 ++++++++---
> > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > Index: b/sound/pci/ali5451/ali5451.c
> > > ===================================================================
> > > --- a/sound/pci/ali5451/ali5451.c
> > > +++ b/sound/pci/ali5451/ali5451.c
> > > @@ -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 (;;) ?
>
> Sure, though I don't see a much value added from this operation..
Heh, it would save one commit I'd do unnecessarily ;)
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 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 <ojbyer@usa.net>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Thanks, applied now.
Takashi
> ---
> sound/pci/ali5451/ali5451.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> Index: b/sound/pci/ali5451/ali5451.c
> ===================================================================
> --- a/sound/pci/ali5451/ali5451.c
> +++ b/sound/pci/ali5451/ali5451.c
> @@ -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;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-08-23 17:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-19 20:09 [PATCH] ali5451: fix timeout handling in snd_ali_{codecs,timer}_ready() Bartlomiej Zolnierkiewicz
2009-08-20 6:33 ` Takashi Iwai
2009-08-23 13:27 ` Bartlomiej Zolnierkiewicz
2009-08-23 17:00 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox