Re: [PATCH] starfire reads irq before pci_enable_device.

Jeff Garzik
2001-02-08T20:34:21Z

Ion Badulescu wrote:
>
> Hi Jeff,
>
> On Wed, 07 Feb 2001 14:57:16 -0500, Jeff Garzik <jgarzik@mandrakesoft.com> wrote:
>
> > Here's the patch I have, against vanilla 2.4.2-pre1, with the
> > pci_enable_device preferred changes included...
>
> Well, I decided to bite the bullet and port my zerocopy starfire
> changes to the official tree, properly ifdef'ed. So here it goes,
> the patch was made against 2.4.1 vanilla and includes all the
> fixes from Jeff and myself that were sent to the list so far.

I would prefer that the zerocopy changes stay in DaveM's external patch
until they are ready to be merged. Zerocopy is still changing and being
actively debugged, so it is possible that we might have to patch
starfire.c again with zerocopy updates, before the final patch makes it
to Linus. Let's wait on zerocopy in the main tree..

> I've also added myself as the starfire maintainer -- I hope
> nobody objects.

If you've got the hardware and time, I'm always happy to see someone
step up .. I must confess that I haven't seen much of your work to
date, however.

> +/*
> + * The ia64 doesn't allow for unaligned loads even of integers being
> + * misaligned on a 2 byte boundary. Thus always force copying of
> + * packets as the starfire doesn't allow for misaligned DMAs ;-(
> + * 23/10/2000 - Jes
> + *
> + * Neither does the Alpha. -Ion
> + */
> +#if defined(__ia64__) || defined(__alpha__)
> +#define PKT_SHOULD_COPY(pkt_len) 1
> +#else
> +#define PKT_SHOULD_COPY(pkt_len) (pkt_len < rx_copybreak)
> +#endif

Note that I have not yet sent this patch onto Linus for a reason...
Here is Don Becker's comment on the subject:

Donald Becker wrote:
> On Tue, 16 Jan 2001, Jeff Garzik wrote:
> > * IA64 support (Jes)
> Oh, and this is completely bogus.
> This isn't a fix, it's a hack that covers up the real problem.
>
> The align-copy should *never* be required because the alignment differs
> between DIX and E-II encapsulated packets. The machine shouldn't crash
> because someone sends you a different encapsulation type!

> @@ -757,14 +931,14 @@
>
> dev->trans_start = jiffies;
> np->stats.tx_errors++;
> - return;
> + netif_wake_queue(dev);
> }

this has not been sent on to linus/alan because, if you do not empty the
Tx ring on tx_timeout, you should check to see if there is space on the
ring before waking the queue. Otherwise corruption/problems occur...

Jeff

--
Jeff Garzik | "You see, in this world there's two kinds of
Building 1024 | people, my friend: Those with loaded guns
MandrakeSoft | and those who dig. You dig." --Blondie