Skip to contentSkip to conversation

char/tpqic02: use poll_event*() interface

RFC PATCH In review Nishanth Aravamudan 2 participants 3 messages Mar 3, 2005 – Mar 5, 2005

Subsystems (2)
Kernel janitors Char and misc drivers
lore
Share & tools
lore

Nishanth Aravamudan

author
#0 lore raw

RFC PATCH 2/5 char/tpqic02: use poll_event*() interface

Hi,

Description: Use new poll_event*() interface to encapsulate sleep-loops.

Signed-off-by Nishanth Aravamudan <[email protected]>
drivers/char/tpqic02.c +4 -12
@@ -527,18 +527,14 @@ static int wait_for_ready(time_t timeout
 		return TE_OK;	/* covers 99.99% of all calls */
 
 	/* Then use schedule() a few times */
-	spin_t = 3;		/* max 0.03 sec busy waiting */
+	spin_t = 3;		/* request max 0.003 sec busy waiting */
 	if (spin_t > timeout)
 		spin_t = timeout;
 	timeout -= spin_t;
 	spin_t += jiffies;
 
 	/* FIXME...*/
-	while (((stat = inb_p(QIC02_STAT_PORT) & QIC02_STAT_MASK) == QIC02_STAT_MASK)  && time_before(jiffies, spin_t))
-	{
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(1);	/* don't waste all the CPU time */
-	}
+	poll_event_timeout(((stat = inb_p(QIC02_STAT_PORT) & QIC02_STAT_MASK) != QIC02_STAT_MASK), 1, spin_t);
 	if ((stat & QIC02_STAT_READY) == 0)
 		return TE_OK;
 

@@ -552,12 +548,8 @@ static int wait_for_ready(time_t timeout
 	TPQDEB({printk("wait_for_ready: additional timeout: %d\n", spin_t);})
 
 	    /* not ready and no exception && timeout not expired yet */
-	while (((stat = inb_p(QIC02_STAT_PORT) & QIC02_STAT_MASK) == QIC02_STAT_MASK) && time_before(jiffies, spin_t)) {
-		/* be `nice` to other processes on long operations... */
-		/* nap 0.30 sec between checks, */
-		/* but could be woken up earlier by signals... */
-		msleep_interruptible(300);
-	}
+	poll_event_interruptible_timeout(((stat = inb_p(QIC02_STAT_PORT) & QIC02_STAT_MASK) != QIC02_STAT_MASK),
+			300, spin_t);
 
 	/* don't use jiffies for this test because it may have changed by now */
 	if ((stat & QIC02_STAT_MASK) == QIC02_STAT_MASK) {

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#1 lore raw
Quoted 20 lines · walter harms
Quoted 15 lines · Nishanth Aravamudan

Hi,

Description: Use new poll_event*() interface to encapsulate sleep-loops.

Signed-off-by Nishanth Aravamudan <[email protected]>

--- 2.6.11-kj-v/drivers/char/tpqic02.c 2005-03-01
23:38:09.000000000 -0800
+++ 2.6.11-kj/drivers/char/tpqic02.c 2005-03-03 10:23:59.000000000 -0800

drivers/char/tpqic02.c +1 -1
@@ -527,18 +527,14 @@ static int wait_for_ready(time_t timeout
 		return TE_OK;	/* covers 99.99% of all calls */
 
 	/* Then use schedule() a few times */
-	spin_t = 3;		/* max 0.03 sec busy waiting */
+	spin_t = 3;		/* request max 0.003 sec busy waiting */
Quoted 7 lines

drivers/char/tpqic02.c static int wait_for_ready(time_t timeout · line 533

Quoted 3 lines
drivers/char/tpqic02.c +0 -0
 	if (spin_t > timeout)
 		spin_t = timeout;
 	timeout -= spin_t;

hi nish,

	the code above looks like:  timeout%=spin_t;

Could be, but one thing at a time :)

Quoted 1 line

the poll*() looks reasonable. perhaps its better to use functions ?

I used macros so that the @condition could be passed in as a parameter (not possible, at least easily, with functions, I don't think).

Quoted 3 lines

As user i would like to know WHY the wait terminated, it may be important to know if a signal or timeout occured, are there ways to find out ?

I certainly could change the macros to respond in the same way that wait_event*() do.

To the list, does that seem like a more reasonable approach?

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Nishanth Aravamudan

drivers/char/tpqic02.c

Quoted 9 lines · Nishanth Aravamudan

Hi,

Description: Use new poll_event*() interface to encapsulate sleep-loops.

Signed-off-by Nishanth Aravamudan <[email protected]>
drivers/char/tpqic02.c +0 -0
...

Since at about half a year, this driver is no longer selectable via Kconfig.

I should resend my patch that removes this driver...

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors