Another OCP enet patch

[David Gibson]

On Mon, May 27, 2002 at 06:25:16PM -0700, Tom Rini wrote:
> On Tue, May 28, 2002 at 10:57:28AM +1000, David Gibson wrote:
> > On Mon, May 27, 2002 at 09:23:23AM -0700, Tom Rini wrote:
> > > On Mon, May 27, 2002 at 02:03:30PM +1000, David Gibson wrote:
> > >
> > > > I realise that logically the OCP enet driver, and the
> > > > consistent_sync() has nothing to do with PCI.  However using the pci.h
> > > > constants seems a better approach than defining new constants with the
> > > > same values, when the switch in consistent_sync() explicitly checks
> > > > against the PCI constants.
> > > >
> > > > In the longer term consistent_sync() itself should be changed not to
> > > > reference the PCI constants - in fact the PCI constants should
> > > > probably be moved and renamed since they have no inherent connection
> > > > with PCI at all.
> > >
> > > At this point I think I should give my 2 cents, so...
> > >
> > > I think we should keep ocp-dma.h around for now (2.4.x timeframe) and
> > > change consistent_sync() to not check for the PCI versions (which I
> > > believe happened just because it's a cut&paste&fix of the ARM code) and
> > > for 2.5 hope that a more general fix comes out..
> >
> > I disagree.  Using the constants from pci.h is the least-wrong simple
> > fix for now - yes it's conceptually wrong, but they're just arbitrary
> > constants, it doesn't do any real damage.
>
> Well, for the moment I think they're less-arbitrary constants than they
> should be, but yes..

Um, I don't see what you mean by "less arbitrary than they should be".

> > Changing consistent_sync() to use its own constants isn't completely
> > trivial, because asm/pci.h calls consistent_sync() with the constants
> > passed as arguments to pci_map_single() et al, assuming they have the
> > correct values.  So, to decouple the consistent_sync() constants from
> > the PCI direction constants we would have to put a switch in every
> > time consistent_sync() is used in asm/pci.h.  The PCI constants could
> > be defined in terms of the non-PCI constants, but that means changing
> > generic code (linux/pci.h).
>
> Well, if DMA_* == PCI_DMA_*, we don't have to do anything.  We put a
> comment saying something like:
> /* The PCI_DMA_* constants have nothing to do with PCI.  Someone should
>  * rename this in 2.5... */
> And while someone could re-number the arbitrary constants, I'd think
> something like that would be questioned before being accepted.

Well, yes, but calling the function with once constant then testing
against another within the function gives me the willies.  If the
constants are changed it will break horribly, and it will give
misleading results if people try to grep for one constant or the
other.

> > If/when we do add non-PCI constants for consistent_sync() they
> > shouldn't go in ocp-dma.h, since they have no more to do with OCP than
> > they do with PCI.  In io.h along with the prototype for
> > consistent_sync() would be a better idea.
>
> I agree with them not really belonging in ocp-dma.h either, but it's
> either a PCI device or an 'Oh Chip Peripheral' of some sort.

Well, for PPC at the moment that's true, but they could be used for
anything.  Constants like this could just as well (but aren't) used
for Sparc SBUS. The constants belong to consistent_sync(), so there is
an obvious place to define them - with its prototype.

> > For 2.5, I think the proper fix is to remove the PCI_DMA_* constants
> > entirely and replace them with some arbitrary constants to define DMA
> > direction.  They can then be used for both PCI and OCP (and sbus,
> > which also uses the PCI constants despite not being PCI).  But that
> > means convincing Linus, of course.
>
> 2.5 still seems to be in break everything mode, so why not send Linus a
> patch which creates <linux/dma.h>, for example, and then do a quick
> search/replace of PCI_DMA_* -> DMA_* and keep the numbers the same even.

Already planning to, sometime soon.

--
David Gibson			| For every complex problem there is a
david at gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.  -- H.L. Mencken
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/