* Re: [KJ] [PATCH] drivers/net/3c523.c : Use of time_after() macro
2005-04-11 10:00 [KJ] [PATCH] drivers/net/3c523.c : Use of time_after() macro Marcelo Feitoza Parisi
@ 2005-04-13 16:12 ` Nish Aravamudan
2005-07-08 18:37 ` [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro Marcelo Feitoza Parisi
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Nish Aravamudan @ 2005-04-13 16:12 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro
2005-04-11 10:00 [KJ] [PATCH] drivers/net/3c523.c : Use of time_after() macro Marcelo Feitoza Parisi
2005-04-13 16:12 ` Nish Aravamudan
@ 2005-07-08 18:37 ` Marcelo Feitoza Parisi
2005-07-08 18:48 ` Nishanth Aravamudan
2005-07-11 18:26 ` Marcelo Feitoza Parisi
3 siblings, 0 replies; 5+ messages in thread
From: Marcelo Feitoza Parisi @ 2005-07-08 18:37 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro
2005-04-11 10:00 [KJ] [PATCH] drivers/net/3c523.c : Use of time_after() macro Marcelo Feitoza Parisi
2005-04-13 16:12 ` Nish Aravamudan
2005-07-08 18:37 ` [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro Marcelo Feitoza Parisi
@ 2005-07-08 18:48 ` Nishanth Aravamudan
2005-07-11 18:26 ` Marcelo Feitoza Parisi
3 siblings, 0 replies; 5+ messages in thread
From: Nishanth Aravamudan @ 2005-07-08 18:48 UTC (permalink / raw)
To: kernel-janitors
[-- 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] drivers/net/3c523.c : Use of time_after macro
2005-04-11 10:00 [KJ] [PATCH] drivers/net/3c523.c : Use of time_after() macro Marcelo Feitoza Parisi
` (2 preceding siblings ...)
2005-07-08 18:48 ` Nishanth Aravamudan
@ 2005-07-11 18:26 ` Marcelo Feitoza Parisi
3 siblings, 0 replies; 5+ messages in thread
From: Marcelo Feitoza Parisi @ 2005-07-11 18:26 UTC (permalink / raw)
To: kernel-janitors
[-- 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
^ permalink raw reply [flat|nested] 5+ messages in thread