Skip to contentSkip to conversation

drivers/net/3c523.c : Use of time_after() macro

PATCH In review Marcelo Feitoza Parisi 3 participants 5 messages Apr 11, 2005 – Jul 11, 2005

Subsystems (2)
Network drivers Kernel janitors
lore
Share & tools
lore

Marcelo Feitoza Parisi

author
#0 lore raw

PATCH 1/1 drivers/net/3c523.c : Use of time_after() macro

Use of time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

Signature · 60 lines
Marcelo Feitoza Parisi            
marcelo at feitoza.com.br
http://marcelo.feitoza.com.br/


Signed-off-by: Marcelo Feitoza Parisi <[email protected]>

--- linux/drivers/net/3c523.c	2005-03-02 04:38:38.000000000 -0300
+++ development/drivers/net/3c523.c	2005-04-09 18:37:48.000000000 -0300
@@ -105,7 +105,8 @@
 #include <linux/mca-legacy.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
-
+#include <linux/jiffies.h>
+ 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -659,7 +660,7 @@ static int init586(struct net_device *de
 
 	s = jiffies;		/* warning: only active with interrupts on !! */
 	while (!(cfg_cmd->cmd_status & STAT_COMPL)) {
-		if (jiffies - s > 30*HZ/100)
+		if (time_after(jiffies, s + 30*HZ/100))
 			break;
 	}
 
@@ -685,7 +686,7 @@ static int init586(struct net_device *de
 
 	s = jiffies;
 	while (!(ias_cmd->cmd_status & STAT_COMPL)) {
-		if (jiffies - s > 30*HZ/100)
+		if (time_after(jiffies, s + 30*HZ/100))
 			break;
 	}
 
@@ -710,7 +711,7 @@ static int init586(struct net_device *de
 
 	s = jiffies;
 	while (!(tdr_cmd->cmd_status & STAT_COMPL)) {
-		if (jiffies - s > 30*HZ/100) {
+		if (time_after(jiffies, s + 30*HZ/100)) {
 			printk(KERN_WARNING "%s: %d Problems while running the TDR.\n", dev->name, __LINE__);
 			result = 1;
 			break;
@@ -799,7 +800,7 @@ static int init586(struct net_device *de
 			elmc_id_attn586();
 			s = jiffies;
 			while (!(mc_cmd->cmd_status & STAT_COMPL)) {
-				if (jiffies - s > 30*HZ/100)
+				if (time_after(jiffies, s + 30*HZ/100))
 					break;
 			}
 			if (!(mc_cmd->cmd_status & STAT_COMPL)) {

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
#1 lore raw
Quoted 24 lines · Marcelo Feitoza Parisi

Use of time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

--

Marcelo Feitoza Parisi
marcelo at feitoza.com.br
http://marcelo.feitoza.com.br/

Signed-off-by Marcelo Feitoza Parisi <[email protected]>
drivers/net/3c523.c 2005-04-09 18:37:48.000000000 -0300 renamed from drivers/net/3c523.c 2005-03-02 04:38:38.000000000 -0300 +2 -1
@@ -105,7 +105,8 @@
 #include <linux/mca-legacy.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
-
+#include <linux/jiffies.h>
+
 #include <asm/uaccess.h>
 #include <asm/processor.h>
 #include <asm/io.h>

drivers/net/3c523.c 2005-04-09 18:37:48.000000000 -0300 static int init586(struct net_device *de · line 663

Quoted 5 lines
drivers/net/3c523.c 2005-04-09 18:37:48.000000000 -0300 renamed from drivers/net/3c523.c 2005-03-02 04:38:38.000000000 -0300 +1 -1
@@ -659,7 +660,7 @@ static int init586(struct net_device *de
 
        s = jiffies;            /* warning: only active with interrupts on !! */
        while (!(cfg_cmd->cmd_status & STAT_COMPL)) {
-               if (jiffies - s > 30*HZ/100)
+               if (time_after(jiffies, s + 30*HZ/100))

It probably makes sense to change "s" to "end_jiffies" and then instead of s=jiffies;
do
end_jiffies = jiffies + 30 * HZ/100;
Then, the if-conditional becomes
if (time_after(jiffies, end_jiffies))

Same with the others (as long as "s" isn't used for anything else, of course).

Thanks,
Nish

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

author
#2 lore raw

PATCH 1/1 drivers/net/3c523.c : Use of time_after macro

In reply to Marcelo Feitoza Parisi

Use of the time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

Signed-off-by Marcelo Feitoza Parisi <[email protected]>
drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300 renamed from drivers/net/3c523.c 2005-07-07 19:11:50.000000000 -0300 +1 -0
@@ -105,6 +105,7 @@
 #include <linux/mca-legacy.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
+#include <linux/jiffies.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>

@@ -658,7 +659,7 @@
 
        s = jiffies;            /* warning: only active with interrupts

on !! */

        while (!(cfg_cmd->cmd_status & STAT_COMPL)) {
-               if (jiffies - s > 30*HZ/100)
+               if (time_after(jiffies, s + 30*HZ/100))
                        break;
        }

@@ -684,7 +685,7 @@

        s = jiffies;
        while (!(ias_cmd->cmd_status & STAT_COMPL)) {
-               if (jiffies - s > 30*HZ/100)
+               if (time_after(jiffies, s + 30*HZ/100))
                        break;
        }

@@ -709,7 +710,7 @@

        s = jiffies;
        while (!(tdr_cmd->cmd_status & STAT_COMPL)) {
-               if (jiffies - s > 30*HZ/100) {
+               if (time_after(jiffies, s + 30*HZ/100)) {
                        printk(KERN_WARNING "%s: %d Problems while
running the TDR.\n", dev->name, __LINE__);
                        result = 1;
                        break;
@@ -798,7 +799,7 @@
                        elmc_id_attn586();
                        s = jiffies;
                        while (!(mc_cmd->cmd_status & STAT_COMPL)) {
-                               if (jiffies - s > 30*HZ/100)
+                               if (time_after(jiffies, s + 30*HZ/100))
                                        break;
                        }
                        if (!(mc_cmd->cmd_status & STAT_COMPL)) {

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

#3 lore raw

Re: [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro

In reply to Marcelo Feitoza Parisi
Quoted 20 lines · Marcelo Feitoza Parisi

Use of the time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

Signed-off-by Marcelo Feitoza Parisi <[email protected]>
drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300 renamed from drivers/net/3c523.c 2005-07-07 19:11:50.000000000 -0300 +1 -0
@@ -105,6 +105,7 @@
 #include <linux/mca-legacy.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
+#include <linux/jiffies.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>

@@ -658,7 +659,7 @@
 
        s = jiffies;            /* warning: only active with interrupts

on !! */

        while (!(cfg_cmd->cmd_status & STAT_COMPL)) {

drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300

Quoted 2 lines
drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300 renamed from drivers/net/3c523.c 2005-07-07 19:11:50.000000000 -0300 +1 -1
-               if (jiffies - s > 30*HZ/100)
+               if (time_after(jiffies, s + 30*HZ/100))

Just personal preference, but it seems like it might be nice to rename "s" to "stop" and do the addition at initialization, e.g. instead of

s = jiffies;

do

stop = jiffies + msecs_to_jiffies(300);

and then the time_after() call becomes

if (time_after(jiffies, stop))

But your code is fine as is...

-Nish

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

author
#4 lore raw

Re: [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro

In reply to Marcelo Feitoza Parisi
Quoted 35 lines · Nishanth Aravamudan
Quoted 20 lines · Marcelo Feitoza Parisi

Use of the time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

Signed-off-by Marcelo Feitoza Parisi <[email protected]>
drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300 renamed from drivers/net/3c523.c 2005-07-07 19:11:50.000000000 -0300 +1 -0
@@ -105,6 +105,7 @@
 #include <linux/mca-legacy.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
+#include <linux/jiffies.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>

@@ -658,7 +659,7 @@
 
       s = jiffies;            /* warning: only active with interrupts

on !! */

       while (!(cfg_cmd->cmd_status & STAT_COMPL)) {

drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300

Quoted 2 lines
drivers/net/3c523.c 2005-07-07 20:32:54.379096144 -0300 renamed from drivers/net/3c523.c 2005-07-07 19:11:50.000000000 -0300 +1 -1
-               if (jiffies - s > 30*HZ/100)
+               if (time_after(jiffies, s + 30*HZ/100))

Just personal preference, but it seems like it might be nice to rename "s" to "stop" and do the addition at initialization, e.g. instead of

s = jiffies;

do

stop = jiffies + msecs_to_jiffies(300);

Quoted 9 lines

and then the time_after() call becomes

if (time_after(jiffies, stop))

But your code is fine as is...

-Nish

Use of the time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

Use of the time_after() macro, defined at linux/jiffies.h, which deal with wrapping correctly and are nicer to read.

Signed-off-by Marcelo Feitoza Parisi <[email protected]>
drivers/net/3c523.c +10 -9
@@ -105,6 +105,7 @@
 #include <linux/mca-legacy.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
+#include <linux/jiffies.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>

@@ -619,7 +620,7 @@
 static int init586(struct net_device *dev)
 {
 	void *ptr;
-	unsigned long s;
+	unsigned long stop;
 	int i, result = 0;
 	struct priv *p = (struct priv *) dev->priv;
 	volatile struct configure_cmd_struct *cfg_cmd;

@@ -656,9 +657,9 @@
 	p->scb->cmd = CUC_START;	/* cmd.-unit start */
 	elmc_id_attn586();
 
-	s = jiffies;		/* warning: only active with interrupts on !! */
+	stop = jiffies + msecs_to_jiffies(300);		/* warning: only active with interrupts on !! */
 	while (!(cfg_cmd->cmd_status & STAT_COMPL)) {
-		if (jiffies - s > 30*HZ/100)
+		if (time_after(jiffies, stop))
 			break;
 	}
 

@@ -682,9 +683,9 @@
 	p->scb->cmd = CUC_START;	/* cmd.-unit start */
 	elmc_id_attn586();
 
-	s = jiffies;
+	stop = jiffies + msecs_to_jiffies(300);
 	while (!(ias_cmd->cmd_status & STAT_COMPL)) {
-		if (jiffies - s > 30*HZ/100)
+		if (time_after(jiffies, stop))
 			break;
 	}
 

@@ -707,9 +708,9 @@
 	p->scb->cmd = CUC_START;	/* cmd.-unit start */
 	elmc_attn586();
 
-	s = jiffies;
+	stop = jiffies + msecs_to_jiffies(300);
 	while (!(tdr_cmd->cmd_status & STAT_COMPL)) {
-		if (jiffies - s > 30*HZ/100) {
+		if (time_after(jiffies, stop)) {
 			printk(KERN_WARNING "%s: %d Problems while running the TDR.\n", dev->name, __LINE__);
 			result = 1;
 			break;

@@ -796,9 +797,9 @@
 			p->scb->cbl_offset = make16(mc_cmd);
 			p->scb->cmd = CUC_START;
 			elmc_id_attn586();
-			s = jiffies;
+			stop = jiffies + msecs_to_jiffies(300);
 			while (!(mc_cmd->cmd_status & STAT_COMPL)) {
-				if (jiffies - s > 30*HZ/100)
+				if (time_after(jiffies, stop))
 					break;
 			}
 			if (!(mc_cmd->cmd_status & STAT_COMPL)) {

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