* [PATCH] tms380tr: fix long delays in initialization
@ 2010-10-03 19:34 Meelis Roos
2010-10-04 3:01 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Meelis Roos @ 2010-10-03 19:34 UTC (permalink / raw)
To: netdev
tms380tr driver tries to use udelay (meaning busy loop) for several half
second delays during hardware initialization. Crazy overly long busy
wait delays mean no delay at all so driver initialization fails without
waiting. Fix it by using msleep() for long delays and leave it to
udelay() for short delays.
Signed-off-by: Meelis Roos <mroos@linux.ee>
---
drivers/net/tokenring/tms380tr.c | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/tokenring/tms380tr.c b/drivers/net/tokenring/tms380tr.c
index aaec637..e6dc2df 100644
--- a/drivers/net/tokenring/tms380tr.c
+++ b/drivers/net/tokenring/tms380tr.c
@@ -177,6 +177,7 @@ static void tms380tr_update_rcv_stats(struct net_local *tp,
unsigned char DataPtr[], unsigned int Length);
/* "W" */
void tms380tr_wait(unsigned long time);
+static void tms380tr_wait_long(unsigned long time);
static void tms380tr_write_rpl_status(RPL *rpl, unsigned int Status);
static void tms380tr_write_tpl_status(TPL *tpl, unsigned int Status);
@@ -1216,20 +1217,19 @@ static void tms380tr_set_multicast_list(struct net_device *dev)
}
/*
- * Wait for some time (microseconds)
+ * Wait for some time (microseconds) - busy wait
*/
void tms380tr_wait(unsigned long time)
{
-#if 0
- long tmp;
-
- tmp = jiffies + time/(1000000/HZ);
- do {
- tmp = schedule_timeout_interruptible(tmp);
- } while(time_after(tmp, jiffies));
-#else
udelay(time);
-#endif
+}
+
+/*
+ * Wait for long time (microseconds) - schedule
+ */
+static void tms380tr_wait_long(unsigned long time)
+{
+ msleep(time / 1000);
}
/*
@@ -1352,9 +1352,9 @@ static int tms380tr_bringup_diags(struct net_device *dev)
int loop_cnt, retry_cnt;
unsigned short Status;
- tms380tr_wait(HALF_SECOND);
+ tms380tr_wait_long(HALF_SECOND);
tms380tr_exec_sifcmd(dev, EXEC_SOFT_RESET);
- tms380tr_wait(HALF_SECOND);
+ tms380tr_wait_long(HALF_SECOND);
retry_cnt = BUD_MAX_RETRIES; /* maximal number of retrys */
@@ -1365,7 +1365,7 @@ static int tms380tr_bringup_diags(struct net_device *dev)
loop_cnt = BUD_MAX_LOOPCNT; /* maximum: three seconds*/
do { /* Inspect BUD results */
loop_cnt--;
- tms380tr_wait(HALF_SECOND);
+ tms380tr_wait_long(HALF_SECOND);
Status = SIFREADW(SIFSTS);
Status &= STS_MASK;
@@ -1384,7 +1384,7 @@ static int tms380tr_bringup_diags(struct net_device *dev)
printk(KERN_INFO "%s: Adapter Software Reset.\n",
dev->name);
tms380tr_exec_sifcmd(dev, EXEC_SOFT_RESET);
- tms380tr_wait(HALF_SECOND);
+ tms380tr_wait_long(HALF_SECOND);
}
} while(retry_cnt > 0);
@@ -1457,7 +1457,7 @@ static int tms380tr_init_adapter(struct net_device *dev)
do {
Status = 0;
loop_cnt--;
- tms380tr_wait(HALF_SECOND);
+ tms380tr_wait_long(HALF_SECOND);
/* Mask interesting status bits */
Status = SIFREADW(SIFSTS);
@@ -1506,7 +1506,7 @@ static int tms380tr_init_adapter(struct net_device *dev)
{
/* Reset adapter and try init again */
tms380tr_exec_sifcmd(dev, EXEC_SOFT_RESET);
- tms380tr_wait(HALF_SECOND);
+ tms380tr_wait_long(HALF_SECOND);
}
}
}
--
1.7.1
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tms380tr: fix long delays in initialization
2010-10-03 19:34 [PATCH] tms380tr: fix long delays in initialization Meelis Roos
@ 2010-10-04 3:01 ` David Miller
2010-10-04 8:39 ` Meelis Roos
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2010-10-04 3:01 UTC (permalink / raw)
To: mroos; +Cc: netdev
From: Meelis Roos <mroos@linux.ee>
Date: Sun, 3 Oct 2010 22:34:30 +0300 (EEST)
> tms380tr driver tries to use udelay (meaning busy loop) for several half
> second delays during hardware initialization. Crazy overly long busy
> wait delays mean no delay at all so driver initialization fails without
> waiting. Fix it by using msleep() for long delays and leave it to
> udelay() for short delays.
>
> Signed-off-by: Meelis Roos <mroos@linux.ee>
You can't use msleep() here because this code can be invoked
from interrupts and thus cannot sleep.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tms380tr: fix long delays in initialization
2010-10-04 3:01 ` David Miller
@ 2010-10-04 8:39 ` Meelis Roos
2010-10-04 16:42 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Meelis Roos @ 2010-10-04 8:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev
> > tms380tr driver tries to use udelay (meaning busy loop) for several half
> > second delays during hardware initialization. Crazy overly long busy
> > wait delays mean no delay at all so driver initialization fails without
> > waiting. Fix it by using msleep() for long delays and leave it to
> > udelay() for short delays.
> >
> > Signed-off-by: Meelis Roos <mroos@linux.ee>
>
> You can't use msleep() here because this code can be invoked
> from interrupts and thus cannot sleep.
I checked these two functions that contain long delays that I changed -
tms380tr_bringup_diags() and tms380tr_init_adapter() - to be called only
from tms380tr_chipset_init() that is only called from tms380tr_open() so
no call paths from interrupts AFAICS. Short delyas from interrupt
context are not changed in any way, they still use udelay().
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tms380tr: fix long delays in initialization
2010-10-04 8:39 ` Meelis Roos
@ 2010-10-04 16:42 ` David Miller
2010-10-04 21:15 ` Meelis Roos
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2010-10-04 16:42 UTC (permalink / raw)
To: mroos; +Cc: netdev
From: Meelis Roos <mroos@linux.ee>
Date: Mon, 4 Oct 2010 11:39:02 +0300 (EEST)
>> > tms380tr driver tries to use udelay (meaning busy loop) for several half
>> > second delays during hardware initialization. Crazy overly long busy
>> > wait delays mean no delay at all so driver initialization fails without
>> > waiting. Fix it by using msleep() for long delays and leave it to
>> > udelay() for short delays.
>> >
>> > Signed-off-by: Meelis Roos <mroos@linux.ee>
>>
>> You can't use msleep() here because this code can be invoked
>> from interrupts and thus cannot sleep.
>
> I checked these two functions that contain long delays that I changed -
> tms380tr_bringup_diags() and tms380tr_init_adapter() - to be called only
> from tms380tr_chipset_init() that is only called from tms380tr_open() so
> no call paths from interrupts AFAICS. Short delyas from interrupt
> context are not changed in any way, they still use udelay().
tms380tr_init_adapter() gets called from tms380tr_chk_irq() which is
invoked from the interrupt handler.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tms380tr: fix long delays in initialization
2010-10-04 16:42 ` David Miller
@ 2010-10-04 21:15 ` Meelis Roos
2010-10-04 21:33 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Meelis Roos @ 2010-10-04 21:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev
> tms380tr_init_adapter() gets called from tms380tr_chk_irq() which is
> invoked from the interrupt handler.
Oops, yes, sorry for repeatedly not noticing it even when I was
explicitly searching for it :(
What is the general course from here? Defer the long reset to a work
queue, or some other kind of background task?
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tms380tr: fix long delays in initialization
2010-10-04 21:15 ` Meelis Roos
@ 2010-10-04 21:33 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-10-04 21:33 UTC (permalink / raw)
To: mroos; +Cc: netdev
From: Meelis Roos <mroos@linux.ee>
Date: Tue, 5 Oct 2010 00:15:17 +0300 (EEST)
>> tms380tr_init_adapter() gets called from tms380tr_chk_irq() which is
>> invoked from the interrupt handler.
>
> Oops, yes, sorry for repeatedly not noticing it even when I was
> explicitly searching for it :(
>
> What is the general course from here? Defer the long reset to a work
> queue, or some other kind of background task?
Yes, very likely a workqueue is the best course of action.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-04 21:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-03 19:34 [PATCH] tms380tr: fix long delays in initialization Meelis Roos
2010-10-04 3:01 ` David Miller
2010-10-04 8:39 ` Meelis Roos
2010-10-04 16:42 ` David Miller
2010-10-04 21:15 ` Meelis Roos
2010-10-04 21:33 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).