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

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

[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 <marcelo@feitoza.com.br>

--- 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
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

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

[Nish Aravamudan]

On 4/11/05, Marcelo Feitoza Parisi <marcelo@feitoza.com.br> wrote:
> 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 <marcelo@feitoza.com.br>
> 
> --- 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))

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
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

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

[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 <marcelo@feitoza.com.br>

--- linux/drivers/net/3c523.c   2005-07-07 19:11:50.000000000 -0300
+++ linux-kj/drivers/net/3c523.c        2005-07-07 20:32:54.379096144 -0300
@@ -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
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

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

[Nishanth Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On 08.07.2005 [15:37:41 -0300], Marcelo Feitoza Parisi wrote:
> 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 <marcelo@feitoza.com.br>
> 
> --- linux/drivers/net/3c523.c   2005-07-07 19:11:50.000000000 -0300
> +++ linux-kj/drivers/net/3c523.c        2005-07-07 20:32:54.379096144 -0300
> @@ -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))

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

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

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

[Marcelo Feitoza Parisi]

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

Nishanth Aravamudan wrote:
> On 08.07.2005 [15:37:41 -0300], Marcelo Feitoza Parisi wrote:
> 
>>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 <marcelo@feitoza.com.br>
>>
>>--- linux/drivers/net/3c523.c   2005-07-07 19:11:50.000000000 -0300
>>+++ linux-kj/drivers/net/3c523.c        2005-07-07 20:32:54.379096144 -0300
>>@@ -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))
> 
> 
> 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
> 
> 
Use of the time_after() macro, defined at linux/jiffies.h, which deal
with wrapping correctly and are nicer to read.

[-- Attachment #2: 3c523.patch --]
[-- Type: text/x-patch, Size: 2143 bytes --]

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 <marcelo@feitoza.com.br>

--- linux/drivers/net/3c523.c	2005-07-07 19:11:50.000000000 -0300
+++ linux-kj/drivers/net/3c523.c	2005-07-11 15:24:08.071329832 -0300
@@ -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)) {

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors