On Tue, 2007-28-08 at 20:28 -0400, jamal wrote:
> On Tue, 2007-28-08 at 17:06 -0700, David Miller wrote:
>
> > Devices need to free packets in a deterministic amount of time, no
> > matter what.
>
> If i understood the driver pointed to - "finite amount of time" spec is
> still met. Seems some interupt is generated (TX_IDLE) when the tx path
> gets idle and this will kick the tx cleanup. Mandeep, correct me if i am
> wrong. If my statement is correct, i wouldnt call this a driver bug;->
>
> Robert, I think i asked you this same question on that piece of code
> once - and iirc you said its because you had to synchronise and make
> sure the packet made it to the wire. In this scenario, the packet has
> made it to the wire - just hasnt been freed. Note that when teaching
> pktgen to do batching, i removed that logic and things work fine.
Hi Jamal,
Here is what the datasheet has to say about TxIdle:
"This event is signaled when the transmit state machine enters the idle state
from a non-idle state. This will happen whenever the state machine encounters
an "end-of-list" condition (NULL link field or a descriptor with OWN clear)."
I interpret this to mean that the interrupt gets generates after a packet
is transferred to the TFIFO on the NIC and the next packet in the ring is
NULL.
This interrupt gets generates less often then TxOK which gets generated
after every completed packet transmit. So I'm thinking that maybe the
driver was written to use this interrupt instead of TxOK for this reason.
Really just my speculation.
Anyway, if I rewrite the driver to use TxOK instead of TxIdle, pktgen
works fine. I'm attaching the patch.
Signed-off-by: Mandeep Singh Baines <[EMAIL PROTECTED]>
---
diff --git a/drivers/net/sis900.c b/drivers/net/sis900.c
index 7c6e480..69e5a57 100644
--- a/drivers/net/sis900.c
+++ b/drivers/net/sis900.c
@@ -1032,7 +1032,7 @@ sis900_open(struct net_device *net_dev)
sis900_set_mode(ioaddr, HW_SPEED_10_MBPS, FDX_CAPABLE_HALF_SELECTED);
/* Enable all known interrupts by setting the interrupt mask. */
- outl((RxSOVR|RxORN|RxERR|RxOK|TxURN|TxERR|TxIDLE), ioaddr + imr);
+ outl((RxSOVR|RxORN|RxERR|RxOK|TxURN|TxERR|TxOK), ioaddr + imr);
outl(RxENA | inl(ioaddr + cr), ioaddr + cr);
outl(IE, ioaddr + ier);
@@ -1557,7 +1557,7 @@ static void sis900_tx_timeout(struct net_device *net_dev)
outl(sis_priv->tx_ring_dma, ioaddr + txdp);
/* Enable all known interrupts by setting the interrupt mask. */
- outl((RxSOVR|RxORN|RxERR|RxOK|TxURN|TxERR|TxIDLE), ioaddr + imr);
+ outl((RxSOVR|RxORN|RxERR|RxOK|TxURN|TxERR|TxOK), ioaddr + imr);
return;
}
@@ -1655,7 +1655,7 @@ static irqreturn_t sis900_interrupt(int irq, void
*dev_instance)
do {
status = inl(ioaddr + isr);
- if ((status & (HIBERR|TxURN|TxERR|TxIDLE|RxORN|RxERR|RxOK)) ==
0)
+ if ((status & (HIBERR|TxURN|TxERR|TxOK|RxORN|RxERR|RxOK)) == 0)
/* nothing intresting happened */
break;
handled = 1;
@@ -1665,7 +1665,7 @@ static irqreturn_t sis900_interrupt(int irq, void
*dev_instance)
/* Rx interrupt */
sis900_rx(net_dev);
- if (status & (TxURN | TxERR | TxIDLE))
+ if (status & (TxURN | TxERR | TxOK))
/* Tx interrupt */
sis900_finish_xmit(net_dev);
@@ -2462,7 +2462,7 @@ static int sis900_resume(struct pci_dev *pci_dev)
sis900_set_mode(ioaddr, HW_SPEED_10_MBPS, FDX_CAPABLE_HALF_SELECTED);
/* Enable all known interrupts by setting the interrupt mask. */
- outl((RxSOVR|RxORN|RxERR|RxOK|TxURN|TxERR|TxIDLE), ioaddr + imr);
+ outl((RxSOVR|RxORN|RxERR|RxOK|TxURN|TxERR|TxOK), ioaddr + imr);
outl(RxENA | inl(ioaddr + cr), ioaddr + cr);
outl(IE, ioaddr + ier);
