Skip to contentSkip to conversation

Use msleep in meye driver

PATCH In review Daniel Drake 3 participants 4 messages Jun 2, 2004 – Jun 3, 2004 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Use msleep in meye driver

Remove meye's definition of wait_ms() and switch to using the new global msleep() function.

Against 2.6.7-rc2

drivers/media/video/meye.c renamed from drivers/media/video/meye.c.orig +11 -21
@@ -473,16 +473,6 @@
 /* MCHIP low-level functions                                                */
 /****************************************************************************/
 
-/* waits for the specified miliseconds */
-static inline void wait_ms(unsigned int ms) {
-	if (!in_interrupt()) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(1 + ms * HZ / 1000);
-	}
-	else
-		mdelay(ms);
-}
-
 /* returns the horizontal capture size */
 static inline int mchip_hsize(void) {
 	return meye.params.subsample ? 320 : 640;
@@ -640,12 +630,12 @@
 		for (j = 0; j < 100; ++j) {
 			if (mchip_delay(MCHIP_HIC_STATUS, MCHIP_HIC_STATUS_IDLE))
 				return;
-			wait_ms(1);
+			msleep(1);
 		}
 		printk(KERN_ERR "meye: need to reset HIC!\n");
 	
 		mchip_set(MCHIP_HIC_CTL, MCHIP_HIC_CTL_SOFT_RESET);
-		wait_ms(250);
+		msleep(250);
 	}
 	printk(KERN_ERR "meye: resetting HIC hanged!\n");
 }
@@ -741,7 +731,7 @@
 	for (i = 0; i < 100; ++i) {
 		if (mchip_delay(MCHIP_HIC_STATUS, MCHIP_HIC_STATUS_IDLE))
 			break;
-		wait_ms(1);
+		msleep(1);
 	}
 }
 
@@ -757,7 +747,7 @@
 	for (i = 0; i < 100; ++i) {
 		if (mchip_delay(MCHIP_HIC_STATUS, MCHIP_HIC_STATUS_IDLE))
 			break;
-		wait_ms(1);
+		msleep(1);
 	}
 	for (i = 0; i < 4 ; ++i) {
 		v = mchip_get_frame();
@@ -799,7 +789,7 @@
 	for (i = 0; i < 100; ++i) {
 		if (mchip_delay(MCHIP_HIC_STATUS, MCHIP_HIC_STATUS_IDLE))
 			break;
-		wait_ms(1);
+		msleep(1);
 	}
 
 	for (i = 0; i < 4 ; ++i) {
@@ -1260,11 +1250,11 @@
 
 	mchip_delay(MCHIP_HIC_CMD, 0);
 	mchip_delay(MCHIP_HIC_STATUS, MCHIP_HIC_STATUS_IDLE);
-	wait_ms(1);
+	msleep(1);
 	mchip_set(MCHIP_VRJ_SOFT_RESET, 1);
-	wait_ms(1);
+	msleep(1);
 	mchip_set(MCHIP_MM_PCI_MODE, 5);
-	wait_ms(1);
+	msleep(1);
 	mchip_set(MCHIP_MM_INTA, MCHIP_MM_INTA_HIC_1_MASK);
 
 	switch (meye.pm_mchip_mode) {
@@ -1349,13 +1339,13 @@
 	mchip_delay(MCHIP_HIC_CMD, 0);
 	mchip_delay(MCHIP_HIC_STATUS, MCHIP_HIC_STATUS_IDLE);
 
-	wait_ms(1);
+	msleep(1);
 	mchip_set(MCHIP_VRJ_SOFT_RESET, 1);
 
-	wait_ms(1);
+	msleep(1);
 	mchip_set(MCHIP_MM_PCI_MODE, 5);
 
-	wait_ms(1);
+	msleep(1);
 	mchip_set(MCHIP_MM_INTA, MCHIP_MM_INTA_HIC_1_MASK);
 
 	if (video_register_device(meye.video_dev, VFL_TYPE_GRABBER, video_nr) < 0) {
#1 lore raw
Quoted 2 lines · Daniel Drake

Remove meye's definition of wait_ms() and switch to using the new global msleep() function.

[...]

Quoted 10 lines
drivers/media/video/meye.c renamed from drivers/media/video/meye.c.orig +0 -8
-static inline void wait_ms(unsigned int ms) {
-	if (!in_interrupt()) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(1 + ms * HZ / 1000);
-	}
-	else
-		mdelay(ms);
-}

From what I see in kernel/timer.c, msleep() cannot be called in

interrupt context, so the in_interrupt() test must stay.

Stelian.

Signature · 1 line
Stelian Pop <[email protected]>    
#2 lore raw
Quoted 3 lines
Quoted 2 lines

From what I see in kernel/timer.c, msleep() cannot be called in

interrupt context, so the in_interrupt() test must stay.

mdelay in irq context is *EVIL* though, and will get all the low latency and audio folks very upset...

#3 lore raw
Quoted 6 lines · Arjan van de Ven
Quoted 3 lines
Quoted 2 lines

From what I see in kernel/timer.c, msleep() cannot be called in

interrupt context, so the in_interrupt() test must stay.

mdelay in irq context is *EVIL* though, and will get all the low latency and audio folks very upset...

Indeed :)

In fact, reading the code again shows that wait_ms() is never called in interrupt context, so the in_interrupt() test was useless.

Daniel's patch is then correct, please apply it.

Or should I add a "Signed-off" line too and resend the patch ?

Stelian.

Signature · 1 line
Stelian Pop <[email protected]>