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, Aug 24, 2002 at 05:09:16PM +0200, Benjamin Herrenschmidt wrote:
> - 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.
we have one special problem on m68k, on some machines the IDE
bus is byteswapped (unrelated to cpu endianness). For historical
and performance reasons data to the HD is by default read and
written in this "wrong" order (thus the bswap/swapdata option)
and special fixup code is used in ide_fix_driveid (see
M68K_IDE_SWAPW). However data returned by IDE_DRIVE_CMD is not
treated in any way, so that eg WIN_SMART data end up in the
wrong order on those machines and this is something I would
like to fix properly.
I figure I would define ata_*_{control,data} to handle special
data resp raw HD data and modify ide_handler_parser to return
specialised interrupt handlers or set some additional flag.
Any thoughts?
Richard
Poke in your own special ide-ops function pointers.
This should have been allowed on a per chipset/channel bases.
Did I miss something?
On Tue, 24 Sep 2002, Richard Zidlicky wrote:
> On Sat, Aug 24, 2002 at 05:09:16PM +0200, Benjamin Herrenschmidt wrote:
>
> > - 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.
>
> we have one special problem on m68k, on some machines the IDE
> bus is byteswapped (unrelated to cpu endianness). For historical
> and performance reasons data to the HD is by default read and
> written in this "wrong" order (thus the bswap/swapdata option)
> and special fixup code is used in ide_fix_driveid (see
> M68K_IDE_SWAPW). However data returned by IDE_DRIVE_CMD is not
> treated in any way, so that eg WIN_SMART data end up in the
> wrong order on those machines and this is something I would
> like to fix properly.
> I figure I would define ata_*_{control,data} to handle special
> data resp raw HD data and modify ide_handler_parser to return
> specialised interrupt handlers or set some additional flag.
>
> Any thoughts?
>
> Richard
>
Andre Hedrick
LAD Storage Consulting Group
>we have one special problem on m68k, on some machines the IDE
>bus is byteswapped (unrelated to cpu endianness). For historical
>and performance reasons data to the HD is by default read and
>written in this "wrong" order (thus the bswap/swapdata option)
>and special fixup code is used in ide_fix_driveid (see
>M68K_IDE_SWAPW). However data returned by IDE_DRIVE_CMD is not
>treated in any way, so that eg WIN_SMART data end up in the
>wrong order on those machines and this is something I would
>like to fix properly.
>I figure I would define ata_*_{control,data} to handle special
>data resp raw HD data and modify ide_handler_parser to return
>specialised interrupt handlers or set some additional flag.
>
>Any thoughts?
You just need to provide your own ide-ops doing the right
thing, I don't think you need to touch ata_*_data if you
properly implement "s" ops {in,out}s{b,w,l} for your hwif.
Ben.
On Mon, Sep 23, 2002 at 05:28:03PM -0700, Andre Hedrick wrote:
>
> Poke in your own special ide-ops function pointers.
> This should have been allowed on a per chipset/channel bases.
I need different transfer functions depending on whether drive
control data(like IDENT,SMART) or HD sectors are to be transfered.
Control data requires byteswapping to correct bus-byteorder
whereas sector r/w has to be raw for compatibility.
So that will require 2 additional iops pointers and some change
in ide_handler_parser or ide_cmd_type_parser to select the
appropriate version depending on the drive command.
Richard
>I need different transfer functions depending on whether drive
>control data(like IDENT,SMART) or HD sectors are to be transfered.
>Control data requires byteswapping to correct bus-byteorder
>whereas sector r/w has to be raw for compatibility.
>
>So that will require 2 additional iops pointers and some change
>in ide_handler_parser or ide_cmd_type_parser to select the
>appropriate version depending on the drive command.
No, it doesn't. There are already separate iops for control
and datas, typically {in,out}{b,w,l} are for control (though
only "b" versions are really useful and {in,out}s{b,w,l} are
for datas.
There are cases where ide_{input,output}_data my try to
"re-invent" the "s" functions with a loop of non-s ones,
but you shouldn't have to care about that case. It might
actually work for you because of your weird wiring, but it's
definitely broken for other BE archs, and so drive->slow
shouldn't be set on anything but x86.
Actually, the whole set of iops could probably be shrunk
down to just {in,out}b for control and {in,out}s{w,l} for
datas. Though we probably want, ultimately, to change that
to some different (higher level ?) kind of abstraction.
Ben.
>>I need different transfer functions depending on whether drive
>>control data(like IDENT,SMART) or HD sectors are to be transfered.
>>Control data requires byteswapping to correct bus-byteorder
>>whereas sector r/w has to be raw for compatibility.
>>
>>So that will require 2 additional iops pointers and some change
>>in ide_handler_parser or ide_cmd_type_parser to select the
>>appropriate version depending on the drive command.
>
>No, it doesn't. There are already separate iops for control
>and datas, typically {in,out}{b,w,l} are for control (though
>only "b" versions are really useful and {in,out}s{b,w,l} are
>for datas.
Oops, sorry, I mis-read you
Well, if you have proper iops that swap for normal datas, then
you can use the "normal" fixup routines for control datas
like ident, the same we use on PPC or other BE archs.
The problem is typiucally the same for everybody here: the
swapping of datas themselves must be done so that you get an
exact image of the datas in memory, then you need additional
fixup to "interpret" some of these (ident, smart, ...)
Ben.
Switch the transport on the fly for the channel based on the OPCODE.
Easy to say harder to do.
Andre Hedrick
LAD Storage Consulting Group
On Tue, 24 Sep 2002, Richard Zidlicky wrote:
> On Mon, Sep 23, 2002 at 05:28:03PM -0700, Andre Hedrick wrote:
> >
> > Poke in your own special ide-ops function pointers.
> > This should have been allowed on a per chipset/channel bases.
>
> I need different transfer functions depending on whether drive
> control data(like IDENT,SMART) or HD sectors are to be transfered.
> Control data requires byteswapping to correct bus-byteorder
> whereas sector r/w has to be raw for compatibility.
>
> So that will require 2 additional iops pointers and some change
> in ide_handler_parser or ide_cmd_type_parser to select the
> appropriate version depending on the drive command.
>
> Richard
>