Re: [PATCH] starfire reads irq before pci_enable_device.

Ion Badulescu
2001-02-08T22:05:35Z

On Thu, 8 Feb 2001, Jeff Garzik wrote:

> Well at least let's do it the Linux Kernel Way(tm): separate out the
> zerocopy stuff such that there are minimal ifdefs in the code... For
> example:
>
> /* add these functions... */
> #ifdef ZEROCOPY
> static inline setup_txrx_rings(...) { /*...*/ }
> #else
> static inline setup_txrx_rings(...) { /*...*/ }
> #endif
>
> then in the code itself, where the TxRx ring setup occurs now (ie. where
> ifdefs exist in the code) simply call the new static inline functions.

Hmm. Ok. I'll think about it. Roughly 1/3 of the driver code will be
duplicated if we go this route with the existing structure. I'll try to
make use of a few helper inline functions which are smaller and can be
ifdef'ed without much code duplication.

> The #ifdef ZEROCOPY code you added is a classic example of the kind of
> code I -remove- from the kernel tree.

It's an issue of maintainer convenience vs. esthetics. And (last but not
least) it's also about other people's ability to easily make changes to
the driver, changes they can understand and test. So while in principle I
agree with you, I'm also beginning to understand Donald Becker's
frustration when others remove backward compatibility from his code.

So let's try to find some middle ground, ok? Your first suggestion sounds
reasonable, and hopefully the fate of the zerocopy stuff will be decided
sooner than later.

Stay tuned, there should be another version coming your way sometime
today...

Thanks,
Ion

--
It is better to keep your mouth shut and be thought a fool,
than to open it and remove all doubt.