> The following improves the performance of NE* clones.
Not to be too harsh on anyone that is trying to help out, but the
above claim is a bit dubious...
> /* This check _should_not_ be necessary, omit eventually. */
> - while ((inb_p(NE_BASE+EN0_ISR) & ENISR_RESET) == 0)
> + while ((inb_p((NE_BASE+EN0_ISR)) & ENISR_RESET) == 0)
Umm, adding extra sets of "( ... )" doesn't gain anything other than
making it more work for me to read through the patch to see what you
are trying to change. There are lots of these in this patch...
> - outb_p(ENISR_RESET, NE_BASE + EN0_ISR); /* Ack intr. */
> + outb_p(ENISR_RESET, (NE_BASE + EN0_ISR)); /* Ack intr. */
See above...
> - the start of a page, so we optimize accordingly. */
> + the start of a page, so we optimize accordingly.i
Ummm, vi typo constitutes a patch? (Okay, we all do that now and again...)
> - /* This *shouldn't* happen. If it does, it's the last thing you'll see */
> - if (ei_status.dmaing) {
> + /* This *shouldn't* happen. If it does, we'll fix it. */
> + if (ei_status.dmaing)
> + {
No, that is written as such because it most likely *will* be the last
thing you see, due to the way the hardware is. If the printk gets to the
screen, then you have done well. Also, putting a { on a newline doesn't
do anything more than make the patch bigger...
> + ne_reset_8390(dev);
Well, a post "Oops, shouldn't have done that - reset the card." looks
like a good idea, I doubt it will buy you a recovery.
> - outb_p(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
> + outb_p(E8390_NODMA+E8390_PAGE0+E8390_START, (NE_BASE + NE_CMD));
brackets... NE_BASE macro vs. nic_base will be obscured by GCC.
> - outb_p(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO);
> + outb_p(sizeof(struct e8390_pkt_hdr), (NE_BASE + EN0_RCNTLO));
brackets...
> - outb_p(0, nic_base + EN0_RCNTHI);
> - outb_p(0, nic_base + EN0_RSARLO); /* On page boundary */
> + outw(0, (NE_BASE + EN0_RSARLO)); /* On page boundary */
Bzzt. Can't do that. You have ne1000 cards to deal with, as well as
the fact that access to the 8390 requires the outb_p() -- see the
datasheets if you are in doubt.
> - outb_p(ring_page, nic_base + EN0_RSARHI);
> + outb_p(ring_page, (NE_BASE + EN0_RSARHI));
More brackets...
> - outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD);
> + outb_p(E8390_RREAD+E8390_START, (NE_BASE + NE_CMD));
More brackets...
> - insw(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)>>1);
> + insw((NE_BASE + NE_DATAPORT), hdr, sizeof(struct e8390_pkt_hdr)>>1);
More brackets...
> - insb(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr));
> + insb((NE_BASE + NE_DATAPORT), hdr, sizeof(struct e8390_pkt_hdr));
More brackets...
> - outb_p(ENISR_RDC, nic_base + EN0_ISR); /* Ack intr. */
> + outb_p(ENISR_RDC, (NE_BASE + EN0_ISR)); /* Ack intr. */
More brackets...
>From here on, you have deleted a bunch of code that is ifdef'd out
anyways. That doesn't buy you anything in speed or size.
> - outb_p(count & 0xff, nic_base + EN0_RCNTLO);
> - outb_p(count >> 8, nic_base + EN0_RCNTHI);
> + outw(count, (NE_BASE + EN0_RCNTLO));
Nope, sorry, same as above. ne1000 cards, and you need the i/o pause.
Hitting an 8390 with back-to-back I/O is a good way to fill my mailbox.
> - outb_p(ring_offset & 0xff, nic_base + EN0_RSARLO);
> - outb_p(ring_offset >> 8, nic_base + EN0_RSARHI);
> + outw(ring_offset, (NE_BASE + EN0_RSARLO));
Same as above.
While I can appreciate your effort, you have to realize that 99.9%
of the ne driver CPU load is in the moving the packet. The individual
read/writes to the 8390 registers account for nothing. (Profile the
driver if you don't believe me, as I have done so.) Furthermore, there
are a ridiculous number of cheap half-broken clone cards out there,
and we have to try and work with all of them.
> -#ifdef NE8390_RW_BUGFIX
> - /* Handle the read-before-write bug the same way as the
> - Crynwr packet driver -- the NatSemi method doesn't work.
> - Actually this doesn't always work either, but if you have
> - problems with your NEx000 this is better than nothing! */
[...]
> -#ifdef NE_SANITY_CHECK
> - /* This was for the ALPHA version only, but enough people have
> - been encountering problems so it is still here. */
Furthermore, deleting code that is already #ifdef'ed out doesn't
get you anything in terms of size or performance. When the next
poor sod has to come along and understand the driver, it may help
him/her out a lot to be able to read that code, even if it is inactive.
Paul.
(a.k.a. current unfortunate sod who deals with ne.c)
> Cheers,
> Dick Johnson
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
> Richard B. Johnson
> Project Engineer
> Analogic Corporation
> Voice : (508) 977-3000 ext. 3754
> Fax : (508) 532-6097
> Modem : (508) 977-6870
> Ftp : ftp@boneserver.analogic.com
> Email : rjohnson@analogic.com, johnson@analogic.com
> Penguin : Linux version 2.1.26 on an i586 machine (66.15 BogoMips).
> Warning : It's hard to remain at the trailing edge of technology.
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-