A few notes taken while adapting the ide-pmac (MMIO) code
to the new layer in -ac:
- Do we really want to keep all those _P versions around ?
A quick grep showed _only_ by the non-portable x86 specific
recovery timer stuff that taps ISA timers (well, I think ports
0x40 and 0x43 and an ISA timer). I would strongly suggest to
get rid of those functions from the hwif, and use directly
the appropriate PIO functon i that timer routine. Also make
sure that timer stuff don't get compiled on anything but
legacy harware where we know those ports mean something.
- In ide-iops, the insw, insl, outsw, outsl functions are
broken for big endian. They should not do byteswap on these,
however, implemeting them with a loop of IN/OUT_BYTE/WORD
will cause byteswapped access on archs like PPC.
The problem is that the macros IN/OUT_BYTE/WORD don't define
non-swapping equivalents that would allow us to correctly
implement the "s" versions.
After much thinking about the above, I came to the conslusion
we probably want to just kill all the IN_BYTE, OUT_BYTE, etc...
macros and use directly inb/outb/etc... in the default PIO
ops defined in ide-iops.c. We would then use the normal arch
provided insw/insl/outsw/outsl functions here as well.
I already added a set of ops for MMIO in there using the normal
readb/writeb/... and for the "multiple" versions, I did loops
using the __raw versions (no byteswap) and a manual io_barrier().
I grep'ed in 2.4, the only archs that use custom IN/OUT_BYTE
macros for IDE are m68k and cris. I'm pretty sure m68k is wrong
and should do like PPC :) In anyway, both should be fixed the
new 'clean' way which is to define a set of access functions
to be stuffed in the HWIF structure. The goal of ide-iops is
to provide defaults for PIO (and MMIO soon as those can be made
fairly generic too).
Also, getting rid of the _P version would make things a lot
easier as well here too.
Any comments ?
Ben.
On Sat, 2002-08-24 at 16:15, Benjamin Herrenschmidt wrote:
> - Do we really want to keep all those _P versions around ?
> A quick grep showed _only_ by the non-portable x86 specific
> recovery timer stuff that taps ISA timers (well, I think ports
> 0x40 and 0x43 and an ISA timer). I would strongly suggest to
I'd like to keep them around for the moment. They should be using
udelay() but thats a general issue with _p inb/outb etc.
> After much thinking about the above, I came to the conslusion
> we probably want to just kill all the IN_BYTE, OUT_BYTE, etc.
Agreed entirely
> Also, getting rid of the _P version would make things a lot
> easier as well here too.
What currently uses the _P versions ?
>> - Do we really want to keep all those _P versions around ?
>> A quick grep showed _only_ by the non-portable x86 specific
>> recovery timer stuff that taps ISA timers (well, I think ports
>> 0x40 and 0x43 and an ISA timer). I would strongly suggest to
>
>I'd like to keep them around for the moment. They should be using
>udelay() but thats a general issue with _p inb/outb etc.
I don't think we need them at all in hwif (see below)
>> After much thinking about the above, I came to the conslusion
>> we probably want to just kill all the IN_BYTE, OUT_BYTE, etc.
>
>Agreed entirely
>
>
>> Also, getting rid of the _P version would make things a lot
>> easier as well here too.
>
>What currently uses the _P versions ?
Ok, I looked and found out that those are used by
- some weird stuff in ide.c tapping IO ports at 0x40 and 0x43 that
I assume is a timer. (Can someone make sure that code don't get used
on anything but legacy x86 ?).
- a couple of interface drivers like cmd640 tapping the PCI config
IO stuffs for probing.
In all cases, I firmly beleive those are way outside of the domain
of application of the hwif-> abstraction. Those are code that knows
it's tapping legacy stuff on a IO port, and can/should directly use
the in/out_p function. I don't think those have anything to do in
hwif. All that should be in hwif is what is needed by the generic
code to tap the IDE registers themselves.
Do you agree ? (I'd love to get rid of them :)
If you agree, I'll send you a patch tomorrow along with the fixing
of ide-pmac that will do
- Remove the _P versions from hwif->iops
- slightly change ide-iops to define both sets of iops, one set
providing PIO ops using directly in/outx & int/outsx, and one
set providing MMIO ops using directly read/writex
Anybody that need different routines for the generic IDE code to
tap the taskfile (or eventually DMA) registers (typically cris
arch) will provide it's own set of routines to hwif. I can probably
fix cris (though I don't know anything about that arch, but the
code seem obvious enough), and i'll rely on you to fix m86k :)
Ben.
I have part of cris fixed
On Sun, 25 Aug 2002, Benjamin Herrenschmidt wrote:
> >> - Do we really want to keep all those _P versions around ?
> >> A quick grep showed _only_ by the non-portable x86 specific
> >> recovery timer stuff that taps ISA timers (well, I think ports
> >> 0x40 and 0x43 and an ISA timer). I would strongly suggest to
> >
> >I'd like to keep them around for the moment. They should be using
> >udelay() but thats a general issue with _p inb/outb etc.
>
> I don't think we need them at all in hwif (see below)
>
> >> After much thinking about the above, I came to the conslusion
> >> we probably want to just kill all the IN_BYTE, OUT_BYTE, etc.
> >
> >Agreed entirely
> >
> >
> >> Also, getting rid of the _P version would make things a lot
> >> easier as well here too.
> >
> >What currently uses the _P versions ?
>
> Ok, I looked and found out that those are used by
>
> - some weird stuff in ide.c tapping IO ports at 0x40 and 0x43 that
> I assume is a timer. (Can someone make sure that code don't get used
> on anything but legacy x86 ?).
>
> - a couple of interface drivers like cmd640 tapping the PCI config
> IO stuffs for probing.
>
> In all cases, I firmly beleive those are way outside of the domain
> of application of the hwif-> abstraction. Those are code that knows
> it's tapping legacy stuff on a IO port, and can/should directly use
> the in/out_p function. I don't think those have anything to do in
> hwif. All that should be in hwif is what is needed by the generic
> code to tap the IDE registers themselves.
>
> Do you agree ? (I'd love to get rid of them :)
>
> If you agree, I'll send you a patch tomorrow along with the fixing
> of ide-pmac that will do
>
> - Remove the _P versions from hwif->iops
> - slightly change ide-iops to define both sets of iops, one set
> providing PIO ops using directly in/outx & int/outsx, and one
> set providing MMIO ops using directly read/writex
>
> Anybody that need different routines for the generic IDE code to
> tap the taskfile (or eventually DMA) registers (typically cris
> arch) will provide it's own set of routines to hwif. I can probably
> fix cris (though I don't know anything about that arch, but the
> code seem obvious enough), and i'll rely on you to fix m86k :)
>
>
> Ben.
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Andre Hedrick
LAD Storage Consulting Group
Yep, just be careful of how to decouple the hwif->iops from procfs for pci
and the general lameness of x86 centric issues.
On 24 Aug 2002, Alan Cox wrote:
> On Sat, 2002-08-24 at 16:15, Benjamin Herrenschmidt wrote:
> > - Do we really want to keep all those _P versions around ?
> > A quick grep showed _only_ by the non-portable x86 specific
> > recovery timer stuff that taps ISA timers (well, I think ports
> > 0x40 and 0x43 and an ISA timer). I would strongly suggest to
>
> I'd like to keep them around for the moment. They should be using
> udelay() but thats a general issue with _p inb/outb etc.
>
> > After much thinking about the above, I came to the conslusion
> > we probably want to just kill all the IN_BYTE, OUT_BYTE, etc.
>
> Agreed entirely
>
>
> > Also, getting rid of the _P version would make things a lot
> > easier as well here too.
>
> What currently uses the _P versions ?
>
Andre Hedrick
LAD Storage Consulting Group