* [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace
2004-07-26 22:41 [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace Zach Brown
@ 2004-07-26 22:57 ` Nishanth Aravamudan
2004-07-26 23:01 ` Zach Brown
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nishanth Aravamudan @ 2004-07-26 22:57 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 419 bytes --]
On Mon, Jul 26, 2004 at 03:41:05PM -0700, Zach Brown wrote:
>
> > I would appreciate any comments from the janitors list.
>
> sure looks ok to me.
>
> > -static void udelay2(void)
> > -{
> > - udelay(2);
> > -}
> > -
> > -static void udelay4(void)
> > -{
> > - udelay(4);
> > -}
>
> wow.
Yeah. Just to make sure, the change from TASK_INTERRUPTIBLE in
sleep125ms() to TASK_UNINTERRUPTIBLE in msleep() is ok?
-Nish
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace
2004-07-26 22:41 [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace Zach Brown
2004-07-26 22:57 ` Nishanth Aravamudan
@ 2004-07-26 23:01 ` Zach Brown
2004-07-27 11:38 ` [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: replace Petr Vandrovec
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Zach Brown @ 2004-07-26 23:01 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 211 bytes --]
> Yeah. Just to make sure, the change from TASK_INTERRUPTIBLE in
> sleep125ms() to TASK_UNINTERRUPTIBLE in msleep() is ok?
I suspect so, but I don't know for sure. I wasn't really involved in
the radio bits.
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: replace
2004-07-26 22:41 [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace Zach Brown
2004-07-26 22:57 ` Nishanth Aravamudan
2004-07-26 23:01 ` Zach Brown
@ 2004-07-27 11:38 ` Petr Vandrovec
2004-07-27 12:21 ` Christoph Hellwig
2004-07-27 14:13 ` [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: repl Petr Vandrovec
4 siblings, 0 replies; 6+ messages in thread
From: Petr Vandrovec @ 2004-07-27 11:38 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]
On 26 Jul 04 at 15:41, Nishanth Aravamudan wrote:
> I would appreciate any comments from the janitors list.
>
> Description: Replaced schedule_timeout() with msleep() to guarantee the
> task delays the desired time.
I do not agree. Delay is here just to make sure that we do not talk
to chip too quickly after sending previous command (it is important
especially for get signal strength; I think that it is not needed
at all in set frequency), and as such it does not matter whether we
wait 100ms, 143ms, or one second. I find busy loop with mdelay
unacceptable.
If your architecture has HZ less than 7, then just replace HZ/7 with
HZ/7 + 1...
Best regards,
Petr Vandrovec
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
>
>
> --- linux-vanilla/drivers/media/radio/radio-sf16fmi.c 2004-06-16 05:20:03.000000000 +0000
> +++ linux-dev/drivers/media/radio/radio-sf16fmi.c 2004-07-02 20:46:57.000000000 +0000
> @@ -89,8 +89,7 @@ static inline int fmi_setfreq(struct fmi
>
> outbits(16, RSF16_ENCODE(freq), myport);
> outbits(8, 0xC0, myport);
> - current->state = TASK_UNINTERRUPTIBLE;
> - schedule_timeout(HZ/7);
> + msleep(143); /* was schedule_timeout(HZ/7) */
> up(&lock);
> if (dev->curvol) fmi_unmute(myport);
> return 0;
> @@ -107,8 +106,7 @@ static inline int fmi_getsigstr(struct f
> val = dev->curvol ? 0x08 : 0x00; /* unmute/mute */
> outb(val, myport);
> outb(val | 0x10, myport);
> - set_current_state(TASK_UNINTERRUPTIBLE);
> - schedule_timeout(HZ/7);
> + msleep(143); /* was schedule_timeout(HZ/7) */
> res = (int)inb(myport+1);
> outb(val, myport);
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: replace
2004-07-26 22:41 [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace Zach Brown
` (2 preceding siblings ...)
2004-07-27 11:38 ` [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: replace Petr Vandrovec
@ 2004-07-27 12:21 ` Christoph Hellwig
2004-07-27 14:13 ` [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: repl Petr Vandrovec
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2004-07-27 12:21 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 477 bytes --]
On Tue, Jul 27, 2004 at 01:38:05PM +0200, Petr Vandrovec wrote:
> I do not agree. Delay is here just to make sure that we do not talk
> to chip too quickly after sending previous command (it is important
> especially for get signal strength; I think that it is not needed
> at all in set frequency), and as such it does not matter whether we
> wait 100ms, 143ms, or one second. I find busy loop with mdelay
> unacceptable.
This is msleep, not mdelay, and _not_ a busywait.
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Kernel-janitors] Re: [PATCH] radio/radio-sf16fmi: repl
2004-07-26 22:41 [Kernel-janitors] Re: [PATCH] radio/radio-maestro: replace Zach Brown
` (3 preceding siblings ...)
2004-07-27 12:21 ` Christoph Hellwig
@ 2004-07-27 14:13 ` Petr Vandrovec
4 siblings, 0 replies; 6+ messages in thread
From: Petr Vandrovec @ 2004-07-27 14:13 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 673 bytes --]
On 27 Jul 04 at 13:21, Christoph Hellwig wrote:
> On Tue, Jul 27, 2004 at 01:38:05PM +0200, Petr Vandrovec wrote:
> > I do not agree. Delay is here just to make sure that we do not talk
> > to chip too quickly after sending previous command (it is important
> > especially for get signal strength; I think that it is not needed
> > at all in set frequency), and as such it does not matter whether we
> > wait 100ms, 143ms, or one second. I find busy loop with mdelay
> > unacceptable.
>
> This is msleep, not mdelay, and _not_ a busywait.
Oops. Sorry for confusion then.
Petr Vandrovec
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread