On Tue, Dec 16, 2003 at 02:08:23PM -0800, Jason Thorpe wrote:
> 
> On Dec 16, 2003, at 1:34 PM, Manuel Bouyer wrote:
> 
> >>	1. Move all of the ATA register access stuff into wdc-specific files.
> >>	   ATA register values are still valid in native SATA (they're really
> >>	   part of the ATA command set), but the actual access needs to be
> >>	   fully abstracted out.
> >
> >I had something like this in mind when I added ATAPI support (this is 
> >why
> >there is a sys/dev/ata/ata.c at the first place), but as we didn't have
> >real hardware at this time, I didn't bother with providing an 
> >abstraction
> >to access the registers values.
> >
> >I guess we would need a specific ata_wdc.c for SATA ?
> 
> It will be different for every controller, since each controller's 
> interface is going to be different in this regard.  Perhaps a library 
> routine that builds the initial command FIS would be useful, but that 
> remains to be seen.
> 
> I guess the analogy is more like the umass_isdata.c stuff, for that 
> USB->ATA dongle (I've never actually seen one of this, but I did run 
> across it while diving into the ATA code to figure out how to handle 
> this stuff).
> 
> I think part of the problem is that "atabus" is in wdc.c, when it 
> really should be in ata.c, and all of the low-level register frobbing 
> should be in ata_wdc.c (where most of it is, but clearly not all of 
> it).

atabus is really very little code. It can be moved anywhere else without
problems. But it may not be worth the trouble to try to share it with
SATA, especially as it has a different probe routine.
However, I don't think ata_wdc.c should have more than is has now. The code
in this file only handles data I/O for ATA drives. No common ATA/ATAPI code
should go there.
The problem is that there are some wdc code in sys/dev/ata/ata.c
The reason is that there is no abstraction between reading the registers
content, and using the values. 
Think more about it, the only function in this file that may make sense for
SATA is ata_get_params(). The others probably do not make sense at all
for SATA, and should probably be moved to sys/dev/ic/wdc.c.
atabus* can probably be moved here, but then we'll need some more callbacks
to wdc.c for this. Hum, and we'll probably need a new data structure
to describe an atabus too. 

Any idea on how ATAPI over SATA will look like ? My guess is that we'll
need an equivalent of atapi_wdc.c too, so I wonder if trying to share the
atabus bits wouldn't be more trouble than it's worth.

> 
> >>	2. The "atabus" layer needs to be changed to support more than 2
> >>	drives
> >>	   per channel.
> >>
> >>	3. The "pciide" layer probably should be changed to support more than
> >>	   2 channels.
> >
> >Both of these shouldn't be hard.
> 
> Yah, I didn't think they would be.
> 
> >Well, I'm not sure I know much about legacy command queueing, exept 
> >that
> >because of the work in imposes in the controller driver, and the 
> >limited
> >number of outstanding commands I'm not sure it's a real win. This is 
> >why it's
> >deep in my todo list and I didn't get around looking at this yet (I'm 
> >more
> >interested in hot-plug, for example :)
> 
> Yah, I'm also quite interested in hot-plug.  The Silicon Image SATA 
> controllers support it, for example (not all SATA controllers support 
> it).
> 
> Native SATA command queueing has the potential of improving performance 
> a lot.  One controller I'm working with now can issue up to 31 commands 
> per root port (i.e. those slots are shared if a port multiplier is 
> used, but still...)

So it's not better than PATA for this (I'd hope that they would have bumped
it to at last 256). However, I guess that the controller now does most of
the work ?

> >daily, so feel free to ask questions.
> 
> Ok, cool.  Be sure that I'll have some :-)  I'll be traveling during 
> Christmas, too, but I have VirtualPC for my laptop, so I'll be able to 
> test code as I hack on it.

Does VirtualPC emulate a SATA controller ?

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 24 ans d'experience feront toujours la difference
--