Hey, somebody's paying attention here! I'm impressed ;-).
We don't need to enable an interrupt in order to have the status bit set by
the chip. Enabling an interrupt basically means that the ISR gets called
when the the status flag goes up.
To put it differently: if you don't enable both TxAbort or TxAborted, you
get to handle those events whenever the ISR is active anyway. So either you
get, say 0x2009 (errors + rxdone) or you enter the ISR with 0x0001 (rxdone)
and while you're doing your first round the chip updates the status
register and next time you hit
while ((intr_status = readw(ioaddr + IntrStatus))) {
you find it's 0x2008 now. No matter what interrupts you enable: as long as
you receive a steady flow of interrupts of any kind, the driver should
still work. Sort of.
Not enabling the TxAborted interrupt is easy to justify: the chip just
failed to send out a frame due to massive collisions. Therefore, we are not
in a hurry to be notified and restart Tx, and as we learned recently it
also takes the chip some time to halt the Tx engine.
Of course that's all a moot point if TxAborted always implies TxAbort:
_that_ interrupt is enabled. I think I'm going to enable TxAborted
regardless. It shouldn't make a difference, and if it did, we're better off
having it enabled.
The contrast between interrupts enabled and handled in the current LK
driver is not pretty, but for most of them it's probably rather a clean-up
than a bug fixing issue if anything. I am currently focusing on what I
suspect to be a problem, so my comments below are mostly based on reading
src and docs, not actual experiments.
> Interrupts referenced in the driver and not listed here are: IntrRxNoBuf,
> IntrRxWakeUp, IntrTxAborted
IMO IntrRxNoBuf should be enabled. If there are no Rx buffers left, we want
to know ASAP and free them.
IntrRxWakeUp has been used to indicate that a magic wake up packet was
received (hence the name). On the VT6102, the same bit is now a "general
purpose interrupt". Whatever that means. Proper support for magic packets
might be desirable later on. Not a problem now.
> Interrupts included here but not used in the driver are:
> IntrRxOverflow, IntrRxDropped.
Since we have problems with Tx, I don't care much about Rx right now. May
be a mistake ;-). For IntrRxDropped, though, we call via_rhine_rx(). We do
use it.
> For example what exactly is the difference between IntrTxAbort and
> IntrTxAborted.
Depends on the docs you read. Currently I tend to think that IntrTxAbort is
a misnomer and I have renamed it IntrTxError in my code.
> I was also puzzled as to why the docs say:
> Transmit Descriptor Underflow for IntrMIIChange
>
> I am talking about the newest VT86C100A docs.
I agree, and I am talking about three different docs. I'd be very curious
where this is coming from, I can't imagine that somebody just pulled it out
of thin air. The name IntrMIIChange and its use in the driver suggest that
the 0x0200 interrupt announces changes in the MII status register.
According to all docs I've checked 0x0200 means a Tx underflow related to a
descriptor (as opposed to 0x0010 which is a Tx underflow related to a
frame).
But even if these suspicions are correct, the impact seems rather small.
The driver takes action based on the MII status register anyway, so worst
case is probably we ignore Tx descriptor underflows (which I don't expect
to be very common).
I'm not sure if it was a good idea to take this back to LKML at this point.
I doubt that the greater public is interested in the gory details of VIA
Rhine programming. I suggest we keep the traffic among the interested
parties and send a patch along with a summary once we have come to a
conclusion or a patch needs wider testing. Comments?
Roger
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/