* (no subject)
@ 2002-04-21 21:16 Ivan G.
2002-04-21 23:02 ` your mail Jeff Garzik
0 siblings, 1 reply; 6+ messages in thread
From: Ivan G. @ 2002-04-21 21:16 UTC (permalink / raw)
To: Urban Widmark; +Cc: LKML
Urban,
About the suggestion to make via_rhine_error handle more interrupts,
enum intr_status_bits {
IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
IntrTxDone=0x0002, IntrTxAbort=0x0008, IntrTxUnderrun=0x0010,
IntrPCIErr=0x0040,
IntrStatsMax=0x0080, IntrRxEarly=0x0100, IntrMIIChange=0x0200,
IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
IntrTxAborted=0x2000, IntrLinkChange=0x4000,
IntrRxWakeUp=0x8000,
IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
};
RxEarly, RxOverflow, RxNoBuf are not handled
(which brings up another question - how should they be handled
and where?? It doesn't seem to me that those should end up in error,
sending CmdTxDemand. )
RxErr, RxWakeUp, RxDropped, RxEmpty call via_rhine_rx
TxAbort, TxUnderrun,PCIErr, StatsMax, MIIChange call via_rhine_error
TxAborted calls via_rgine_tx
The others don't look like errors.
Martin Eriksson,
The reason my message said PCI Error and not unhandled
is because it specifies a specific interrupt - IntrPCIErr.
(basically the onle one that's left unhandled that can call via_rhine_error)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: your mail
2002-04-21 21:16 Ivan G.
@ 2002-04-21 23:02 ` Jeff Garzik
2002-04-21 23:25 ` [PATCH] Via-rhine minor issues Ivan G.
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Garzik @ 2002-04-21 23:02 UTC (permalink / raw)
To: Ivan G.; +Cc: Urban Widmark, LKML
On Sun, Apr 21, 2002 at 03:16:40PM -0600, Ivan G. wrote:
> Urban,
>
> About the suggestion to make via_rhine_error handle more interrupts,
>
> enum intr_status_bits {
> IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
> IntrTxDone=0x0002, IntrTxAbort=0x0008, IntrTxUnderrun=0x0010,
> IntrPCIErr=0x0040,
> IntrStatsMax=0x0080, IntrRxEarly=0x0100, IntrMIIChange=0x0200,
> IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
> IntrTxAborted=0x2000, IntrLinkChange=0x4000,
> IntrRxWakeUp=0x8000,
> IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
> };
>
> RxEarly, RxOverflow, RxNoBuf are not handled
> (which brings up another question - how should they be handled
> and where?? It doesn't seem to me that those should end up in error,
> sending CmdTxDemand. )
*blink* I had not noticed that.
All drivers actually need to handle RxNoBufs and RxOverflow, assuming
they have similar meaning to what I'm familiar with on other chips.
The chip may recover transparently, but one should be at least aware of
them.
RxEarly you very likely do -not- want to handle...
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Via-rhine minor issues
2002-04-21 23:02 ` your mail Jeff Garzik
@ 2002-04-21 23:25 ` Ivan G.
2002-04-22 19:33 ` Urban Widmark
0 siblings, 1 reply; 6+ messages in thread
From: Ivan G. @ 2002-04-21 23:25 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML
Sorry, I forgot the subject line for last mail
Message has no subject. Garzik's reply is Re: Your Mail.
As for the Interrupts:
Actually, RxNoBuf is handled by calling via_rhine_rx
but not enabled when setting interrupt mask. My patch will fix that.
However, RxOverflow is never handled at all and neither is RxEarly.
Nor are they enabled when setting interrupt mask (patch enables).
How should RxOverflow be handled?
Should I call via_rhine_rx, like other errors do? - add IntrRxOverflow (and
possibly RxEarly)
if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf))
via_rhine_rx(dev);
How should PCIErr be handled?
Other drivers say:
/* Hmmmmm, it's not clear how to recover from PCI faults. */
> > RxEarly, RxOverflow, RxNoBuf are not handled
> > (which brings up another question - how should they be handled
> > and where?? It doesn't seem to me that those should end up in error,
> > sending CmdTxDemand. )
>
> *blink* I had not noticed that.
>
> All drivers actually need to handle RxNoBufs and RxOverflow, assuming
> they have similar meaning to what I'm familiar with on other chips.
> The chip may recover transparently, but one should be at least aware of
> them.
>
> RxEarly you very likely do -not- want to handle...
>
> Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Via-rhine minor issues
2002-04-21 23:25 ` [PATCH] Via-rhine minor issues Ivan G.
@ 2002-04-22 19:33 ` Urban Widmark
2002-04-23 4:07 ` Ivan G.
0 siblings, 1 reply; 6+ messages in thread
From: Urban Widmark @ 2002-04-22 19:33 UTC (permalink / raw)
To: Ivan G.; +Cc: Jeff Garzik, LKML
On Sun, 21 Apr 2002, Ivan G. wrote:
> As for the Interrupts:
> Actually, RxNoBuf is handled by calling via_rhine_rx
> but not enabled when setting interrupt mask. My patch will fix that.
>
> However, RxOverflow is never handled at all and neither is RxEarly.
> Nor are they enabled when setting interrupt mask (patch enables).
>
> How should RxOverflow be handled?
The public docs don't say.
You should probably add them to the list of reasons to call
via_rhine_error, and let them be caught by the "wicked" error rule. A
reason to use a term that doesn't try to be specific is that we don't
really know what is causing the event.
Before your patch IntrTxUnderrun would raise the tx threshold AND issue a
CmdTxDemand. After it will only do the first. Do you know that this is an
improvement?
Does merging this error handling change into the main kernel really help
you test other things for your timeout problem? Can't you just include
this bit in your other work?
/Urban
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Via-rhine minor issues
2002-04-22 19:33 ` Urban Widmark
@ 2002-04-23 4:07 ` Ivan G.
2002-04-23 13:44 ` Urban Widmark
0 siblings, 1 reply; 6+ messages in thread
From: Ivan G. @ 2002-04-23 4:07 UTC (permalink / raw)
To: Urban Widmark; +Cc: LKML
> The public docs don't say.
>
That is precisely the problem.
4 missing interrupts in the interrupt mask, 1 additional interrupt
which Jeff Garzik says all drivers should handle and the via-rhine doesn't.
2 interrupts handled quite differently in the linuxfet driver.
1 wicked error message of which I've seen 3 different versions and none of
them makes much sense to me.
It is lack of documentation.
The tech sheets do not explain what causes the interrupts in detail, how to
handle the interrupts in detail. They rather provide the bit address of the
interrupt which is good to know, but not enough. What resources did Donald
Becker use when he was writing this?
Perhaps I should ask him.
Or do you know a good document on such things?
> You should probably add them to the list of reasons to call
> via_rhine_error, and let them be caught by the "wicked" error rule. A
> reason to use a term that doesn't try to be specific is that we don't
> really know what is causing the event.
Well, most Rx errors are handled in via_rhine_rx.
The "Wicked" error rule doesn't do anything besides a CmdTxDemand.
Is this correct handling for, let's say RxOverflow? ..Again, facing the
overwhelming lack of documentation :) (see above paragraph)
Yes, we do know what's causing the event.
We know exactly which interrupt.
We just don't know how to handle it.
> Does merging this error handling change into the main kernel really help
> you test other things for your timeout problem? Can't you just include
> this bit in your other work?
>
> /Urban
Ok not really.
Apparently this change is controversial and I'll leave it out of my patch
until decided what to do. I would just like the kernel driver to be less
buggy and I wanted to contribute.
Plus, I've been making too many changes to my copy. It would be easier to
work over a clean kernel driver. I would have less things to keep track of
that have been changed.
---------------------------
While we're talking about bugs, here's a (possibly ignorant) question:
Do Rhine-III's have a place in the following? :
#ifdef USE_MEM
static void __devinit enable_mmio(long ioaddr, int chip_id)
{
int n;
if (chip_id == VT86C100A) {
/* More recent docs say that this bit is reserved ... */
n = inb(ioaddr + ConfigA) | 0x20;
outb(n, ioaddr + ConfigA);
} else if (chip_id == VT6102) {
n = inb(ioaddr + ConfigD) | 0x80;
outb(n, ioaddr + ConfigD);
}
}
#endif
-----------------------------
Well, apart from things which are not precisely clear how to fix,
would you like to me to submit any portions of my patch for inclusion at all?
A little warning: as far as testing goes - my card stalls without the fixes,
and it stalls with the fixes :) Yet, they seem simple enough to offer for
inclusion.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Via-rhine minor issues
2002-04-23 4:07 ` Ivan G.
@ 2002-04-23 13:44 ` Urban Widmark
0 siblings, 0 replies; 6+ messages in thread
From: Urban Widmark @ 2002-04-23 13:44 UTC (permalink / raw)
To: Ivan G.; +Cc: LKML
On Mon, 22 Apr 2002, Ivan G. wrote:
> The tech sheets do not explain what causes the interrupts in detail, how to
> handle the interrupts in detail. They rather provide the bit address of the
> interrupt which is good to know, but not enough. What resources did Donald
> Becker use when he was writing this?
>
> Perhaps I should ask him.
He's the one who wrote:
"Recovery for other fault sources not known."
I have a feeling he didn't have any other sources, except that he had
written a whole bunch of other (similar) drivers before.
Some things you can try to guess:
RxEarly - An interrupt is sent when the chip begins to receive data. if
the driver wants to do something when that happens then it
should enable it. The current driver doesn't so it shouldn't be
enabled.
Or something completely different.
> Well, most Rx errors are handled in via_rhine_rx.
> The "Wicked" error rule doesn't do anything besides a CmdTxDemand.
> Is this correct handling for, let's say RxOverflow? ..Again, facing the
> overwhelming lack of documentation :) (see above paragraph)
One option is not to do anything.
Another to catch the event and call via_rhine_error on it. That will make
it print the "wicked" message and do a CmdTxDemand. Does it hurt to make a
CmdTxDemand when you don't really need it?
If you have a reason to think it hurts, then separate the events that you
do CmdTxDemand on:
if (intr_status & ~IntrRxOverflow)
writew(CmdTxDemand ...);
(which will still do a CmdTxDemand in all cases it used to do one, but
not if the interrupt was just a IntrRxOverflow. I think.)
And the same for the other interrupt bits that weren't enabled.
> Plus, I've been making too many changes to my copy. It would be easier to
> work over a clean kernel driver. I would have less things to keep track of
> that have been changed.
Nothing prevents you from having more than one copy of the driver in your
local tree(s). Since the driver is just one file that works ok, make a
copy of it when you have a version you "like". Then you can look back
later and compare.
> While we're talking about bugs, here's a (possibly ignorant) question:
> Do Rhine-III's have a place in the following? :
Probably. You need to check if their docs also enable MMIO by flipping
that bit, then you could change the "chip_id == VT6102" part to be just an
else.
> Well, apart from things which are not precisely clear how to fix,
> would you like to me to submit any portions of my patch for inclusion at all?
The wait_for_reset thing is clearly a bug, and the cur_tx-1 is probably
right too because you want the number it was queued as. So yes, send them
to Jeff.
/Urban
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-04-23 13:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-04-21 21:16 Ivan G.
2002-04-21 23:02 ` your mail Jeff Garzik
2002-04-21 23:25 ` [PATCH] Via-rhine minor issues Ivan G.
2002-04-22 19:33 ` Urban Widmark
2002-04-23 4:07 ` Ivan G.
2002-04-23 13:44 ` Urban Widmark
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).